Re: Remove the IRC channels

2024-05-05 Thread Stefan Sperling
On Sun, May 05, 2024 at 11:20:28AM +0200, Daniel Sahlberg wrote:
> What about this new topic for #svn?
> 
> [[[
> The Apache® Subversion® version control system (
> https://subversion.apache.org/) | Read the book: http://www.svnbook.org/ |
> FAQ: https://subversion.apache.org/faq.html | This channel has limited use,
> if you have questions please ask on the mailing list
> https://subversion.apache.org/mailing-lists#users-ml
> ]]]
> 
> And for #svn-dev:
> 
> [[[
> Development of Apache® Subversion® (https://subversion.apache.org/) | This
> channel has limited use, if you have questions please ask on the mailing
> list https://subversion.apache.org/mailing-lists#dev-ml
> ]]]

Reads well. Fine with me!


Re: Remove the IRC channels

2024-05-04 Thread Stefan Sperling
On Sat, May 04, 2024 at 09:24:58PM +0200, Daniel Sahlberg wrote:
> Hi,
> 
> I’m personally not an IRC user but I try to keep an eye on the IRC logs.
> For personal reasons I haven’t had time to do since the start of the year
> but I spent some time tonight to browse the archives.
> 
> Since January there were somewhere between 5 and 10 persons asking
> questions and NO ONE got a timely reply. On the other hand we have had a
> slow but steady stream of questions on users@ and they have received
> replies from different members of the community.
> 
> I think the lack of replies on IRC reflects badly on the community. I’m not
> able to put any energy into following the IRC channels and I cannot ask
> anyone else either. But I think the mailing lists could absorb these
> questions and give timely answers.
> 
> For this reason, I suggest that we discontinue the IRC channels (remove
> them from libera.chat) or at least change the topic to indicate that they
> are no longer “official” and to refer all questions to the mailing lists.
> The website should of course be updated accordingly.
> 
> Kind regards
> 
> Daniel Sahlberg

I am not sure whether outright removing (aka "dropping") our Libera
channels from ChanServ is a good idea. Anyone could re-register a
dropped channel and squat on the name and/or impersonate the project.

Given this, I suppose the #svn users channel should be marked as
unmaintained in the topic. Redirecting questions to the mailing
list via the topic line should work well enough.

The #svn-dev channel might still be useful for quick communication
during bursts of increased project activity. I would keep it around.

Cheers,
Stefan


Re: 1.14.4 release

2024-03-11 Thread Stefan Sperling
On Sun, Mar 10, 2024 at 07:59:38PM -0400, Nathan Hartman wrote:
> There is at least a week before I can begin working on it, so there is
> time for the dev community to respond.

I will be around to help with testing/signing. Thanks for picking this up!


Re: Changing the permission checks in libsvn_subr/io.c

2024-01-10 Thread Stefan Sperling
On Wed, Jan 10, 2024 at 09:44:51AM +0100, Johan Corveleyn wrote:
> Interesting discussion. I agree it should at least be documented, and
> perhaps be made a bit more clear from the output of 'revert' (but not
> sure how far we can go without breaking compat). Changing the current
> behavior is probably a more risky move, given the maturity of SVN and
> backwards compatibility etc.

Adding new notification types should not cause compatibility concerns.
If adding new notifications breaks other software which reads command
line client output than this other software has a bug: It should have
been ignoring unknown lines of output in the first place.

API users would likewise simply need to catch the new notification type
in a switch-like statement and should also be ignoring unknown values.

I would only see a compatibility problem if an existing notification
about an important event no longer appears.

In the past we have made significant changes to output from commands,
such as when tree conflict detection was added in 1.6. That by itself
has not resulted in any problems, as far as I know.


Re: Subversion 1.14.3 up for testing/signing

2023-12-13 Thread Stefan Sperling
On Sat, Dec 09, 2023 at 11:00:00AM -0700, Nathan Hartman wrote:
> The 1.14.3 release artifacts are now available for testing/signing.
> Please get the tarballs from
>   https://dist.apache.org/repos/dist/dev/subversion
> and add your signatures there.
> 
> Thanks!

Summary: +1 to release

Tested: [bdb | fsfs] x [ra_local | ra_svn | ra_serf]
swig bindings
javahl bindings

Test results: All passed.

Platform: OpenBSD 7.4 amd64

Dependencies:
bdb:4.7.25
GNU-iconv:  1.15
apr:1.7.0
apr-util:   1.6.1
httpd:  2.4.37
serf:   1.3.10
cyrus-sasl: 2.1.28
sqlite: 3390400
lz4:1.7.5
libssl: LibreSSL 3.8.2
swig:   4.1.0
python: 3.10.8
perl:   5.36.1
ruby:   2.7.4
java:   17.0.9

Signatures:

subversion-1.14.3.tar.gz
-BEGIN PGP SIGNATURE-

iF0EABECAB0WIQSxzxBgoek00Z6G1tbl0wJz9Z0l8AUCZXmlZwAKCRDl0wJz9Z0l
8IIwAKD2wK/rnl2UQKrfOky2LViOATy5CQCgh88jUXwB4fzAlwrhuOe8t/Y8LRM=
=P7zH
-END PGP SIGNATURE-


subversion-1.14.3.tar.bz2
-BEGIN PGP SIGNATURE-

iF0EABECAB0WIQSxzxBgoek00Z6G1tbl0wJz9Z0l8AUCZXmlZwAKCRDl0wJz9Z0l
8NfyAKCNzMW+mM42nhMPKsTTGMmBalSahwCgm7pKfbUqmf8YbXT5bFdEBlkJ9I4=
=CUBD
-END PGP SIGNATURE-


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Stefan Sperling
On Sat, Nov 11, 2023 at 05:08:28PM +0100, Daniel Sahlberg wrote:
> ne_openssl.o isn't built, the build fails already when running configure on
> neon.

Can you show the cofigure log? E.g. via paste.apache.org?
I am also in #svn-dev on Libera IRC if you want to chat there.


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Stefan Sperling
On Sat, Nov 11, 2023 at 04:34:07PM +0100, Stefan Sperling wrote:
> On Sat, Nov 11, 2023 at 04:23:52PM +0100, Daniel Sahlberg wrote:
> > As for the original problem, updating to the latest version of the Neon
> > library doesn't help, I still get the same linking error. I''m sure there
> > is something fishy with my Debian install...
> 
> Which libssl is your build trying to use?
> 
> There is nothing in Makefile.svn that would try to pick a particular
> version of OpenSSL. It just expects that a compatible version is
> already installed.
> 
> Maybe you have multiple versions installed and that confuses things?
 
What is the output of this command on your system (in the ~/svn
build dir):

 nm objdir/neon-0.32.5/src/ne_openssl.o | grep -i SSL_library_init

This doesn't print anything for me. If this shows the symbol
on your system that would indicate a wrong OPENSSL_VERSION macro
being used during the build.


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Stefan Sperling
On Sat, Nov 11, 2023 at 04:23:52PM +0100, Daniel Sahlberg wrote:
> As for the original problem, updating to the latest version of the Neon
> library doesn't help, I still get the same linking error. I''m sure there
> is something fishy with my Debian install...

Which libssl is your build trying to use?

There is nothing in Makefile.svn that would try to pick a particular
version of OpenSSL. It just expects that a compatible version is
already installed.

Maybe you have multiple versions installed and that confuses things?


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Stefan Sperling
On Sat, Nov 11, 2023 at 04:07:52PM +0100, Stefan Sperling wrote:
> On Sat, Nov 11, 2023 at 04:00:58PM +0100, Stefan Sperling wrote:
> > On Sat, Nov 11, 2023 at 03:53:09PM +0100, Stefan Sperling wrote:
> > > If neon requests OPENSSL_API_COMPAT 0x1000L and also calls
> > > OPENSSL_init_ssl() then that is a bug in neon.
> > 
> > The neon version used by Makefile.svn is quite old.
> > Newer releases are available here: https://notroj.github.io/neon/
> > You could try using the latest 0.32.5 release instead of 0.30.2.
> > That should deal just fine with newer versions of OpenSSL.
>  
> This should be enough to get started:

Building with this I got an error during 'make install' in neon.
I could fix that by installing xmlto and calling the 'make docs'
target explicitly.

I have committed these changes. The update was long overdue in any case.


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Stefan Sperling
On Sat, Nov 11, 2023 at 04:00:58PM +0100, Stefan Sperling wrote:
> On Sat, Nov 11, 2023 at 03:53:09PM +0100, Stefan Sperling wrote:
> > If neon requests OPENSSL_API_COMPAT 0x1000L and also calls
> > OPENSSL_init_ssl() then that is a bug in neon.
> 
> The neon version used by Makefile.svn is quite old.
> Newer releases are available here: https://notroj.github.io/neon/
> You could try using the latest 0.32.5 release instead of 0.30.2.
> That should deal just fine with newer versions of OpenSSL.
 
This should be enough to get started:

Index: Makefile.svn
===
--- Makefile.svn(revision 1913716)
+++ Makefile.svn(working copy)
@@ -118,7 +118,7 @@
 APR_UTIL_VER   = 1.6.1
 PCRE_VER   = 8.41
 HTTPD_VER  = 2.4.37
-NEON_VER   = 0.30.2
+NEON_VER   = 0.32.5
 SERF_VER   = 1.3.10
 CYRUS_SASL_VER = 2.1.28
 SQLITE_VER = 3390400
@@ -157,7 +157,7 @@
 SHA256_${GNU_ICONV_DIST} = 
ccf536620a45458d26ba83887a983b96827001e92a13847b45e4925cc8913178
 SHA256_${PCRE_DIST} = 
244838e1f1d14f7e2fa7681b857b3a8566b74215f28133f14a8f5e59241b682c
 SHA256_${HTTPD_DIST} = 
aa97a834a32d51974be8d8a013b561e28d327387cb1da2c3c2762acd0146aabd
-SHA256_${NEON_DIST} = 
db0bd8cdec329b48f53a6f00199c92d5ba40b0f015b153718d1b15d3d967fbca
+SHA256_${NEON_DIST} = 
4872e12f802572dedd4b02f870065814b2d5141f7dbdaf708eedab826b51a58a
 SHA256_${CYRUS_SASL_DIST} = 
7ccfc6abd01ed67c1a0924b353e526f1b766b21f42d4562ee635a8ebfc5bb38c
 SHA256_${SQLITE_DIST} = 
f31d445b48e67e284cf206717cc170ab63cbe4fd7f79a82793b772285e78fdbb
 SHA256_${LIBMAGIC_DIST} = 
694c2432e5240187524c9e7cf1ec6acc77b47a0e19554d34c14773e43dbbf214
@@ -212,8 +212,7 @@
 APR_UTIL_URL   = https://svn.apache.org/repos/asf/apr/apr-util
 PCRE_URL   = 
https://downloads.sourceforge.net/project/pcre/pcre/$(PCRE_VER)/$(PCRE_DIST)
 HTTPD_URL  = https://archive.apache.org/dist/httpd/$(HTTPD_DIST)
-#NEON_URL  = http://webdav.org/neon/$(NEON_DIST)
-NEON_URL   = http://ftp.openbsd.org/pub/OpenBSD/distfiles/$(NEON_DIST)
+NEON_URL   = https://notroj.github.io/neon/$(NEON_DIST)
 SERF_URL   = https://svn.apache.org/repos/asf/serf/tags/$(SERF_VER)
 SQLITE_URL = https://www.sqlite.org/2022/$(SQLITE_DIST)
 CYRUS_SASL_URL = 
https://github.com/cyrusimap/cyrus-sasl/releases/download/cyrus-sasl-${CYRUS_SASL_VER}/$(CYRUS_SASL_DIST)


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Stefan Sperling
On Sat, Nov 11, 2023 at 03:53:09PM +0100, Stefan Sperling wrote:
> If neon requests OPENSSL_API_COMPAT 0x1000L and also calls
> OPENSSL_init_ssl() then that is a bug in neon.

The neon version used by Makefile.svn is quite old.
Newer releases are available here: https://notroj.github.io/neon/
You could try using the latest 0.32.5 release instead of 0.30.2.
That should deal just fine with newer versions of OpenSSL.


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Stefan Sperling
On Sat, Nov 11, 2023 at 02:22:55PM +0100, Daniel Sahlberg wrote:
> Hi,
> 
> I'm separating this out to a new thread since it doesn't relate to the
> upcoming release, but I'm doing the work trying to reproduce Nathan's
> problem.
> 
> I've installed Debian Bullseye (on Hyper-V, but that shouldn't matter I
> think). When I try to do the build, I end up having trouble building neon.
> The error message is "could not find library containing SSL_library_init".
> I have checked that the package libssl-dev is installed (with version
> 1.1.1w-0+deb11u1).
> 
> I've tried a very simple file:
> #include 
> int main(void)
> {
> SSL_library_init();
> return 0;
> }
> 
> $ gcc -lssl -lcrypto -DOPENSSL_API_COMPAT=0x1000L ssl.c
> [..] undefined reference to OPENSSL_init_ssl [...]
> 
> This makes sense, since ssl.h defines SSL_library_init() as
> OPENSSL_init_ssl(0, NULL) (protected by an #if checking OPENSSL_API_COMPAT
> < 0x1010L).
> 

This function first appeared in OpenSSL 1.1.0.
Which means trying to call it on earlier version of OpenSSL is an
expected failure.

> I've tried changing the order of the libraries (-lcrypto -lssl) with the
> same result.
> 
> I'm sure I'm missing something simple but I've spent the better half of a
> day figuring out what.

If neon requests OPENSSL_API_COMPAT 0x1000L and also calls
OPENSSL_init_ssl() then that is a bug in neon.


Re: New release

2023-10-13 Thread Stefan Sperling
On Fri, Oct 13, 2023 at 08:43:59AM +0200, Daniel Sahlberg wrote:
> Hi,
> 
> There are quite a number of improvements waiting to be released. Can we
> muster the energy to do a new release?
> 
> In trunk there are a lot of changes that warrant a 1.15, but before doing
> that I think we should also go back to the discussion of changing the /
> adding a checksum algorithm in the WC. That deserves a separate thread and
> I'm half way through summarising the previous discussions so I'd like to
> hold back 1.15 for the moment.
> 
> Still in 1.14 there have been a number of bugfixes that might be good to
> get released. Maybe doing 1.14.3 first could set us up to do 1.15 a little
> later?

The first problem to solve before the ball starts rolling would be
finding a release manager. I don't have enough spare time to play RM
this time around but I would support the RM as far as my time allows for.

Doing 1.14.3 first sounds like a good plan. This would help potential
new releases managers to get bootstrapped into the process. A major
release tends to involve a little bit more effort because some new problems
may only show up on specific operating system platforms during -rc testing.

Regards,
Stefan


Re: mailer.py

2023-10-13 Thread Stefan Sperling
On Fri, Oct 13, 2023 at 12:42:27AM -0500, Greg Stein wrote:
> Hey all,
> 
> So I'm looking at incorporating a couple key svn-mailer (by Andre Malo)
> features into mailer.py. Specifically, the body-length limit and mail
> encoding, and in turn deferring to viewvc links to replace what would be
> large emails.
> 
> The py3 work was done by stsp and futatuki, so I ask them specifically, and
> the community in general: do we have any guarantees on *internal*
> interfaces of the mailer.py script? Do we know of any users that import
> that script and use its functions/classes? In other words, is there an
> implied API to maintain guarantees for?
>
> If the answer is "no", then I'd prefer to change the dataflow for rendering
> and its write(_binary) mechanism.
> 
> Thoughts?

I have only seen cases where mailer.py is invoked with its command line
via a hook script.

And I would say if anyone reached into internals they will well be
able to deal with updates that break things for them and adapt their
code. It's not going to be a hugely complicated effort for them.
We never promised such compatibility in the first place, as far as I know.

Cheers,
Stefan


Re: Possibly missing pointer dereference in parse-diff.c

2023-01-09 Thread Stefan Sperling
On Mon, Jan 09, 2023 at 11:19:48AM +0100, Johannes von Rotz wrote:
> Hello
> 
> I was trying to compile subversion with the HP ANSI C compiler on HP-UX
> yesterday, which complained about the if-statement in question requiring a
> scalar value or something. Unfortunately, I'm unable to recite the specific
> error message, since I'm currently at work (Ahem...)
> 
> It seems to me that there is a missing pointer dereference in that
> if-statement, but i might be wrong about that. Feel free to ignore this...
> 
> Cheers, J.

Thank you Johannes, this has been committed. Your fix is indeed correct.

> Index: subversion/libsvn_diff/parse-diff.c
> ===
> --- subversion/libsvn_diff/parse-diff.c   (revision 1906480)
> +++ subversion/libsvn_diff/parse-diff.c   (working copy)
> @@ -1006,7 +1006,7 @@ parse_pretty_mergeinfo_line(svn_boolean_t *found_m
>  }
>(*number_of_reverse_merges)--;
>  }
> -  else if (number_of_forward_merges > 0) /* forward merges */
> +  else if (*number_of_forward_merges > 0) /* forward merges */
>  {
>if (patch->reverse)
>  {



Re: Subversion 1.10.0 end-of-life

2022-04-28 Thread Stefan Sperling
On Thu, Apr 28, 2022 at 01:29:43PM +, Daniel Shahaf wrote:
> Stefan Sperling wrote on Thu, 28 Apr 2022 09:55 +00:00:
> > I think it would be better to have such details spelled out in English
> > in a manner that is easy to understand for anyone, with illustrating
> > examples, instead of (or in addition to) mathematical notation that
> > requires abstract thinking to figure out.
> 
> I'm unable to interpret this charitably.

Fine, I'll best just shut up then.


Re: Subversion 1.10.0 end-of-life

2022-04-28 Thread Stefan Sperling
On Wed, Apr 27, 2022 at 11:43:02PM -0400, Nathan Hartman wrote:
> On Wed, Apr 27, 2022 at 4:56 PM Daniel Sahlberg
>  wrote:
> >
> > Den ons 27 apr. 2022 kl 21:02 skrev Daniel Shahaf :
> >>
> >> As to the general rule, I think we're missing a piece: the overlap
> >> period.  We should say something along the lines of "Every LTS release
> >> will be supported for at least Y years, or until M months after the
> >> release of another LTS .0, whichever comes later.".
> >
> >
> > +1 to have an overlap period.
> >
> > Y = 4, M = 3? Or M = 6?
> 
> 
> I'm also +1 to have an overlap period, and the idea of "at least Y
> years, or until M months after the release of another LTS .0,
> whichever comes later" seems quite reasonable to me.

Shouldn't this say "whichever comes earlier"?
Otherwise, the M months rule would never apply in case we release more
than one LTS line within Y years, right? Would we then end up fully
supporting several lines of LTS releases?

Example with Y=4:
release 1.15.0 in year 1 (support 1.15)
release 1.16.0 in year 2 (support 1.15, 1.16)
release 1.17.0 in year 3 (support 1.15, 1.16, 1.17)
release 1.18.0 in year 4 (support 1.15, 1.16, 1.17, 1.18)
release 1.19.0 in year 5 (support 1.16, 1.17, 1.18, 1.19)
...

I think it would be better to have such details spelled out in English
in a manner that is easy to understand for anyone, with illustrating
examples, instead of (or in addition to) mathematical notation that
requires abstract thinking to figure out.


Re: Subversion 1.10.0 end-of-life

2022-04-26 Thread Stefan Sperling
On Mon, Apr 25, 2022 at 10:05:58PM +0200, Daniel Sahlberg wrote:
> Hi,
> 
> According to the Roadmap, How we plan releases[1], 1.10.0 is a LTS release
> that will receive support for 4 years. According to the News archive[2],
> 1.10.0 was released 2018-04-13.
> 
> 1.10.0 was released approximately two months before the transition to the
> LTS support policy and I have not been able to dig out what was promised
> previously.

Before LTS releases, the 2 most recent lines of releases were supported.
The most recent one would receive all types of bug fixes, the second one
would receive security or data-corruption fixes only.

I think it would make sense to go back to our old release policy.

As far as I understand, the goal of the LTS policy was to publish 
releases more quickly to get features tested earlier by users and
gather feedback before such features would be set in stone as part
of an LTS release. This policy was invented at the same time as the
shelving feature. I don't know for sure but I believe this feature and
the involvement of a particular sponsor with some time-to-market constraints
are part of the reason why a release policy change was suggested in the first
place. The shelving feature was developed in multiple iterations and was
shipped in several drafts, as planned. But it never made it beyond an
experimental status, and given there has not been further development effort
on this feature for quite some time I don't believe this will change.
No other feature I am aware of has since adopted the development and
release model which was introduced along with the shelving feature.

The old release policy had its own share of problems. Essentially,
every release was an LTS release, but the support time window for a
release was not announced in advance. In practice, due to relatively slow
pace of development, it worked out well (just look at the pacing of past
release in the CHANGES file, it was not unreasonable).
The most significant drawback of the old policy at the time was that the
incentive to ship a release with features in early stages of development
was very low. It was difficult to decide when the code on trunk was ready
to be released. Every released feature had to be supported with full
backwards compatibility going forward.
This caused some friction, again with sponsors who had time-to-market
constraints. The most prominent case occurred in the 1.5 cycle during
which merge-tracking was developed (see here for an essay about this:
http://www.hyrumwright.org/papers/floss2009.pdf). Ultimately, merge-tracking
was shipped in a de-facto "experimental" state, and users were not all happy
about this. Later releases had to correct several design-level problems and
implementation bugs. Had the current LTS/non-LTS release split existed at
that time, some of these issues might have been avoided before appearing
as part of an LTS release and be supported forever.

In any case, our current policy only works when it is actually implemented
as intended, and this is not happening. In my opinion, our old release policy
is more suited for the current state of things. Feature development does not
need to be rushed, we don't have sponsors anymore who promise features to
their clients and then come back to the project to ask about our progress
on the next release. And our userbase seems to be more interested in a
stable long-term support platform than in trying out experimental features.

Going back to the old policy would mean that 1.10 would be supported
until 1.15 comes out. At which point only 1.15 and 1.14 would be supported.

Cheers,
Stefan


Re: [PATCH] Make sure resolving a conflict with `svn_wc_conflict_choose_merged' can't break the workqueue by referencing a non-existing file

2022-04-21 Thread Stefan Sperling
On Thu, Apr 21, 2022 at 08:25:32PM +0300, Sergey Raevskiy wrote:
> I've noticed that in some cases a working copy can become inoperable
> after merge text conflict resolution with `svn_wc_conflict_choose_merged'
> option.
> 
> The problem can occur in the following scenario:
> 
>   1. There is a text conflict, that is resolved with
>  `svn_wc_conflict_choose_merged' option and specifying some
>  path as MERGED_FILE.  According to the API it can be any file,
>  let's say "D:\merged.txt".
> 
>   2. Previous step adds a FILE_INSTALL item into workqueue with a
>  reference to "D:\merged.txt" and then runs the workqueue.
> 
>   3. An error happens when running the workqueue.
>  For example a working file cannot be overwritten because
>  of 'Access denied' error.
> 
>   4. The "D:\merged.txt" file gets deleted for some reason.
> 
>   5. At this point the working copy is inoperable and cannot be
>  fixed by 'svn cleanup', because workqueue contains absolute
>  path to non-existing file ("D:\merged.txt").
> 
> I've attached a patch with fix for this problem.  The patch contains a
> simplified regression test that reproduces problem by returning
> non-existing path from resolver callback.
> 
> Log message:
> [[[
> Make sure resolving a conflict with `svn_wc_conflict_choose_merged' can't
> break the workqueue by referencing a non-existing file
> 
> * subversion/libsvn_wc/conflicts.c
>   (build_text_conflict_resolve_items): Create private copy of MERGED_FILE
>contents in case of `svn_wc_conflict_choose_merged' is specified.
>Doing that to avoid referencing a potentially absent file in FILE_INSTALL
>workqueue record.
> 
> Patch by: sergey.raevskiy{_AT_}visualsvn.com
> ]]]

Thanks, Sergey! Your fix makes sense to me. One question below:

> Index: subversion/libsvn_wc/conflicts.c
> ===
> --- subversion/libsvn_wc/conflicts.c  (revision 1900114)
> +++ subversion/libsvn_wc/conflicts.c  (working copy)
> @@ -1706,9 +1706,49 @@ build_text_conflict_resolve_items(svn_skel_t **wor
> good to use". */
>case svn_wc_conflict_choose_merged:
>  {
> -  install_from_abspath = merged_file
> -  ? merged_file
> -  : local_abspath;
> +  if (merged_file)
> +{
> +  /* Create private copy of merged MERGED_FILE contents to
> + avoid referencing a potentially absent file in FILE_INSTALL
> + workqueue record. */
> +
> +  const char *temp_dir_abspath;
> +  const char *temp_file_abspath;
> +  svn_stream_t *merged_contents;
> +  svn_stream_t *tmp_contents;
> +  apr_file_t *file;
> +
> +  SVN_ERR(svn_io_file_open(, merged_file,
> +   APR_READ, APR_OS_DEFAULT,
> +   scratch_pool));
> +  merged_contents = svn_stream_from_aprfile2(file, FALSE,
> + scratch_pool);
> +
> +  SVN_ERR(svn_wc__db_temp_wcroot_tempdir(_dir_abspath, db,
> + local_abspath,
> + scratch_pool,
> + scratch_pool));
> +
> +  SVN_ERR(svn_stream_open_unique(_contents,
> + _file_abspath,
> + temp_dir_abspath,
> + svn_io_file_del_none,
> + scratch_pool, scratch_pool));
> +
> +  SVN_ERR(svn_stream_copy3(merged_contents, tmp_contents,
> +   cancel_func, cancel_baton,
> +   scratch_pool));
> +
> +  install_from_abspath = temp_file_abspath;

Should we copy file permission bits as well as content?
For example, with the svn_io_copy_perms() function?

Or perhaps this entire copy operation could be done with svn_io_copy_file(),
and with the copy_perms parameter set to TRUE? If this is possible then the
code would be a lot simpler.

> +static svn_error_t *
> +test_resolve_choose_merged_non_existing_path(const svn_test_opts_t *opts,
> + apr_pool_t *pool)
> +{

We could also verify matching file system permission bits in this test.


Re: Call for release signatures

2022-04-07 Thread Stefan Sperling
On Thu, Apr 07, 2022 at 10:07:15AM -0400, Nathan Hartman wrote:
> > We should have enough signatures according to ASF rules (need 3 signatures
> > by PMC members), but not for our own cross-platform testing requirements
> > (there is only one windows signature so far).
> >
> > Should we fail to meet our own higher standard, we can fall back on ASF
> > rules, meaning we could release with the signatures we have available now.
> >
> > While not required this time around, the RM could in principle count
> > their own signature against the 3 required by ASF.
> >
> > I don't mean to discourage further testing/signing; if you can test these
> > releases on Windows or other platforms which have not yet been covered in
> > some way, this would be very welcome. Testing from people outside the PMC
> > is also always welcome, even though a signature won't be useful in this
> > case.
> >
> > Cheers,
> > Stefan
> >
> I'm a little bit confused... Per HACKING we need three +1 votes from PMC
> members, at least one of which is for each platform (Windows and Unix). We
> have already met this requirement for both 1.14.2 and 1.10.8, haven't we?

Oh, so the rules have been updated? Sorry for the confusion. I still had
the old rules in mind, which required 3 votes for each *nix and windows.


Re: Call for release signatures

2022-04-07 Thread Stefan Sperling
On Thu, Apr 07, 2022 at 09:37:08AM -0400, Mark Phippard wrote:
> Just a reminder, the 1.10.8 and 1.14.2 releases are posted and
> available for testing and signatures. Please try to get them completed
> by this Sunday.
> 
> The plan is to make the release available on Tuesday April 12.
> 
> Thanks
> 
> Mark
> 

We should have enough signatures according to ASF rules (need 3 signatures
by PMC members), but not for our own cross-platform testing requirements
(there is only one windows signature so far).

Should we fail to meet our own higher standard, we can fall back on ASF
rules, meaning we could release with the signatures we have available now.

While not required this time around, the RM could in principle count
their own signature against the 3 required by ASF.

I don't mean to discourage further testing/signing; if you can test these
releases on Windows or other platforms which have not yet been covered in
some way, this would be very welcome. Testing from people outside the PMC
is also always welcome, even though a signature won't be useful in this case.

Cheers,
Stefan


Re: Subversion 1.14.2 up for testing/signing

2022-04-04 Thread Stefan Sperling
On Sat, Apr 02, 2022 at 09:28:15AM -0400, Mark Phippard wrote:
> The 1.14.2 release artifacts are now available for testing/signing.
> 
> Please get the tarballs from
>   https://dist.apache.org/repos/dist/dev/subversion
> and add your signatures there.

Summary: +1 to release

Tested: [bdb | fsfs] x [ra_local | ra_svn | ra_serf]
swig bindings
javahl bindings

Test results: All passed.

Platform: OpenBSD 7.0 amd64

Dependencies:
bdb:4.7.25
GNU-iconv:  1.15
apr:1.7.0
apr-util:   1.6.1
httpd:  2.4.37
serf:   1.3.9
cyrus-sasl: 2.1.25
sqlite: 3160200
lz4:1.7.5
libssl: LibreSSL 3.4.1
swig:   4.0.2
python: 3.7.5
perl:   5.32.1
ruby:   2.7.4
java:   11.0.12

Signatures:

subversion-1.14.2.tar.gz
-BEGIN PGP SIGNATURE-

iQEzBAABCAAdFiEEi8Ta4MWk1l9ARAEHT326qZpZuXMFAmJK9doACgkQT326qZpZ
uXNQvQf/bLxThHqK1pbHkej4vKyzM+siMyQPoYN5JiGFKs8Mxht/Bf+6Gq9Kopr6
XMRl3vDU7WWW9E3sS5ygH0JKkm+BaF41ahPzirM7m+dDNb4UdSpWcT7WjG+O3DAV
lMzojsshu9T9/KRcjIAO0el/cCyrOFmj2LXxRgHVYBCozJU8uFl+971og7hNeO0v
R+NYtdUo9AzvUzbDNrFRsViisyfrjgxD7LQw1V74tiNwPPuPmIUDmktqzWM0AJ2v
syPfSNx+pkLlt0ej8c9reiJC+ngduQQEuBwqljsxDf3MTS2dQkxq3IeVlB38RyRx
tFvtSbzWKuMGWRpi1QmgdKHnU/c4ug==
=6zzL
-END PGP SIGNATURE-

subversion-1.14.2.tar.bz2
-BEGIN PGP SIGNATURE-

iQEzBAABCAAdFiEEi8Ta4MWk1l9ARAEHT326qZpZuXMFAmJK9doACgkQT326qZpZ
uXNxqAf+PtDGZD4Qd1fJCns6eK18lIU8xBMy1Q+TxRp/RqtnSg075osJGevvCsmZ
syn8JcoPFzc+EzX94bOLAFbcnxC+FZTyAiMMCcnmLO+uGUfMMGxUTiXV8Vkc9VHJ
WDKqKfzjDbmSGBmb4nKMyChbpfWFyw5INnPxxCA4GDjYqavlA+RsDx2efwtD3zWz
SNa0Ww8aQyWD9hSY8MU/iVNkajEE1fVIxHjAuHlvyTYPZr7jEavpQoMLMiKKiJJG
mLpEpPvfud92D4TaxKK/nFbdn2XKfEVZ61/0hstj7B/BZnufBFsWVNmU6fXbfnGg
RgLNgPYXMeozlwq2d6eh340NKcSrCQ==
=hC7u
-END PGP SIGNATURE-


Re: Subversion 1.10.8 up for testing.signing

2022-04-04 Thread Stefan Sperling
On Sat, Apr 02, 2022 at 09:27:18AM -0400, Mark Phippard wrote:
> The 1.10.8 release artifacts are now available for testing/signing.
> 
> Please get the tarballs from
>   https://dist.apache.org/repos/dist/dev/subversion
> and add your signatures there.
> 
> Thanks!

Summary: +1 to release

Tested: [bdb | fsfs] x [ra_local | ra_svn | ra_serf]
swig bindings
javahl bindings

Test results: All passed.

Platform: OpenBSD 7.0 amd64

Dependencies:
bdb:4.7.25
GNU-iconv:  1.15
apr:1.7.0
apr-util:   1.6.1
httpd:  2.4.37
serf:   1.3.9
cyrus-sasl: 2.1.25
sqlite: 3160200
lz4:1.7.5
libssl: LibreSSL 3.4.1
swig:   3.0.12
python: 3.7.5
perl:   5.32.1
ruby:   2.7.4
java:   1.8.0_302

Signatures:

subversion-1.10.8.tar.gz
-BEGIN PGP SIGNATURE-

iQEzBAABCAAdFiEEi8Ta4MWk1l9ARAEHT326qZpZuXMFAmJK9eMACgkQT326qZpZ
uXPIhAgAzR/pWjYONFN30R31/bJx10iodiERbjuS3G8ZqEQQAs1nT4vSHSH1pgub
dyoF/q3+iDbpvgqeigMm5Fp+iOchDU/8bG9b5rWdXkeEn88r4de2lw+ioEywcmv0
CMa7nrxwnEPpd+IVBQ/BmWA9TqZhQK2IIRZ9trljX8ssva14J6YaJ7PhSaUWttgs
Qvf76mfW7N5t/2cW9jbdUEm9H+dIUwir2M/AwK3Utmb3MPSw6PCEFL+H7tCY1G6p
qPkkdxrzyglt5Lt/D2S3+nuCOzhmczQwG1IF6FNbyvW2Bf30xJPit54+3ISivS4k
yDJn4bplBOhtYnD4cEJjmhwirCm18A==
=qWf/
-END PGP SIGNATURE-

subversion-1.10.8.tar.bz2
-BEGIN PGP SIGNATURE-

iQEzBAABCAAdFiEEi8Ta4MWk1l9ARAEHT326qZpZuXMFAmJK9eMACgkQT326qZpZ
uXOlAggAnhDjYRDBmaembQAOMEo7GEuoCVSWnmTGOfktmrqZMPEce7nDrOOqcQW1
mxz6eruHf4R30CC/uKRsQbq9j5qDP/N/MbPAOlWUvkZiMWwbwMuiEOoMm1JYlG/s
DbLSF3SUwS8r5ofoBVv+AN3vg1//XnOTAH/Wh5HVVwOnyNL08yMVIqVyZ3Nfz8Vx
gLhybFK1hHIBpf5rad6B6IFabxuqV5C9G4TGJu7wZa4NIW7eRRY+eqO88D70T06D
n3dMoonkoCabpuiUVqMB9q66HCnEZVryf4878hfVvYQFjU34zZAXEQsv3Q0VxJaS
hluoyxXWqIJCxKlJGKuCQhzZrgKtGg==
=Zytq
-END PGP SIGNATURE-


Re: [PATCH] limit diff effort to fix performance issue

2022-04-01 Thread Stefan Sperling
On Fri, Apr 01, 2022 at 06:08:14PM +0200, Johan Corveleyn wrote:
> I suppose the "modified for better performance" refers to some
> optimisations done by Morten Kloster, who then later submitted the
> patch adding this comment in r1128862. His optimisations were more
> related to the LCS algorithm (further reducing the problem space, and
> providing early exits in certain cases or some such).
> 
> The core LCS algorithm in libsvn_diff/lcs.c was from way before my
> time, and according to 'svn blame' was mainly written by Sander
> Striker. I don't understand it fully either :-), but I always assumed
> it was basically some clever implementation of Myers' algorithm.

Yes, that is my understanding as well.

I believe this performance problem has always existed.
I have already tried reverting optimizations made over time, and it did
not make any difference with the problematic test data I have.


Re: [PATCH] limit diff effort to fix performance issue

2022-04-01 Thread Stefan Sperling
On Fri, Apr 01, 2022 at 05:04:49PM +0200, Johan Corveleyn wrote:
> Yes, I suppose this is the case: Patience feeds different (smaller)
> things to LCS. Because, as far as I understand, Myers' way of
> calculating the LCS is fundamentally "somewhat" quadratic (according
> to the Myers paper from 1986 [1], titled "An O(ND) Difference
> Algorithm and Its Variations").
> 
> That is: O(ND) where N = the sum of the lengths of both sides of the
> diff and D = the size of the minimal diff. That's why eliminating the
> common prefix and suffix (added as an optimization years ago) is
> useful, it makes N much smaller.
> 
> Now, in cases where both texts are huge (even after prefix-suffix
> stripping), but the minimal diff is small, the algorithm should in
> theory be fast (because D is small). Just not sure what is going wrong
> then in our variation of this algorithm. Or have we implemented
> another algorithm than the one described by Myers?

SVN diff is allegedly "based on the approach described by ... Meyers (sic),
[...]  but has been modified for better performance."
I guess the modifications refer to prefix/suffix scanning,
because this description dates from r1128862.

For gameoftrees Neels re-implemented Myers from scratch, using the original
paper and some articles found on the web as a reference. This code looks
very different to SVN diff. But those differences could be due to
implementation style. I cannot judge that. Neels code makes more sense to
me and is exhaustively commented. The SVN diff code is not nearly as
readable from my point of view.

Neels diff code was developed stand-alone in a separate Git repository,
with the intention that it could be re-used outside of gameoftrees:
https://git.gameoftrees.org/gitweb/?p=diff.git;a=summary
If someone eventually ends up looking at rewriting SVN diff's code from
scratch, then this code would be a good starting point (and the licence
is compatible). Neels is an inactive SVN committer, but can be reached
and might be able to provide assistance if required.


Re: Review CHANGES for release

2022-04-01 Thread Stefan Sperling
On Fri, Apr 01, 2022 at 10:16:41AM -0400, Mark Phippard wrote:
> I think we are all set for tomorrow. I updated CHANGES on trunk for
> both releases and backported to the branches. Feel free to review and
> make edits if you see fit. I will just go with whatever is on the
> branches tomorrow AM.
> 
> Now, back to $DAYJOB

There have been some exceptions to this, but generally we've been trying
to fit change log entries within 80 coloumns. I have made some edits
in that direction in r1899491. There are some tricks we use for this,
for instance, if there is an issue number then we don't need to list
revision numbers.

I don't believe any important information was dropped. But if you see
an entry that is now missing information, please feel free to fix it
to your liking.

Cheers,
Stefan


Re: [PATCH] limit diff effort to fix performance issue

2022-04-01 Thread Stefan Sperling
On Fri, Apr 01, 2022 at 04:17:45PM +0200, Johan Corveleyn wrote:
> - Perhaps the fundamental diff algorithm in SVN is fine, but it has a
> performance bug / low-level inefficiency? I think that should be
> explored first, because it might fix most of this problem without
> requiring a discussion about the precise output, and without huge
> efforts of rewrites.

I myself find the current SVN diff code very difficult to make sense of.
Maybe someone smarter than me (like Sander Striker?) could figure this out.

> - I think a lot of other diff tools out there have an "effort-limit",
> like the one you proposed (like --speed-large-files, aka
> "use-heuristic" for GNU diff [1], [2], which might be the default
> behaviour, dunno). I'm guessing 'git diff' has one too. So agreed,
> perhaps we need one too. Just not entirely sure how high we should put
> the limit, and what we have to communicate around "diff output might
> have changed / might not be a minimal diff" (and whether we still need
> an option to fallback to "go over the limit").

The only way I found to terminate SVN diff code early leaves a
giant ugly diff chunk at the bottom of the diff.
Other diff implementations do not have this issue. And I don't know
how this issue could be fixed in the current code.

> - Patience diff might also be an option (though it might have its own
> set of problems and peculiar edge cases, I don't know). It might even
> be possible to reuse part of the current code for this (Not sure this
> requires a complete rewrite. Does it still require a LCS algorithm at
> its core, after picking apart the problem space? I haven't studied it
> enough to know how it works internally). In this case too there is the
> back-compat vs behavioral-change discussion that will take time to
> settle (Do we change the default algorithm / output? Do we make it
> optional, or do we already have too manu knobs? ...).

Yes, roughly, Patience diff involves two algorithms, the grouping of
lines along similar-line boundaries performed by Patience, and an
LCS for parts of the files which Patience cannot deal with by itself.

But LCS needs to complete its work before Patience can do its thing,
and vice-versa. For a given input, each algorithm might run multiple
times, switching whenever the current algorithm cannot make any process.
So if LCS still has a fundamental performance issue for some inputs,
adding Patience diff probably won't help. It could help in cases where
LCS is now fed with different inputs as determined by Patience, such
that bad inputs for LCS are avoided. But I don't know if that would
always be the case.


Re: [PATCH] limit diff effort to fix performance issue

2022-04-01 Thread Stefan Sperling
On Fri, Apr 01, 2022 at 12:44:24PM +0200, Johan Corveleyn wrote:
> On Tue, Jun 8, 2021 at 5:58 PM Johan Corveleyn  wrote:
> > On Tue, Jun 8, 2021 at 3:24 PM Stefan Sperling  wrote:
> > > On Tue, Jun 08, 2021 at 02:57:34PM +0200, Johan Corveleyn wrote:
> > > > Okay, I focused on the first revision causing the annotate to differ,
> > > > and with some debug logging:
> > > > - p went up to 139
> > > > - length[0]=1942, length[1]=1817
> > > >
> > > > Now, 1942 lines on the left and 1817 on the right doesn't seem all
> > > > that many (that's what remains after prefix-suffix stripping). I see
> > > > no reason 'svn diff' shouldn't be able to calculate a minimal diff for
> > > > those sizes in a reasonable amount of time. So if p can go up to such
> > > > a "cost" for such "reasonable" lenghts, I imagine we should put the
> > > > actual limit much higher. I suppose we can just as well set "min_cost"
> > > > to 1024 or even higher. 64 is way too low.
> > >
> > > Thanks!
> > > I have set the value back to at least 1024 with this new version of patch.
> >
> > Hmmm, apparently I'm still running into the limit. Even with 8192 I'm
> > hitting it at least once. The actual diff there is not really pretty,
> > but it's an actual manual change made by some developer years ago, and
> > the "minimal diff" is more or less readable / understandable. The log
> > message is something like "Group sections related to multimedia
> > module", and it shuffles around a lot of xml sections to group them
> > together.
> >
> > In this case, length[0]==length[1]==46780. Pretty big for the LCS, but
> > not humongous. The value of 'p' goes up to 8279.
> >
> > With the limit set to 16384, I'm not hitting it, and the annotate
> > comes out as with the original.
> >
> > Now, I'm a bit puzzled why limit==8192 already takes 18s in your
> > tests. In my situation, with the above diff, I get:
> > limit==8192: 3.3s
> > limit==16384: 3.5s
> >
> > (that's including downloading both versions from the server over https)
> >
> > So I'm wondering why it takes so long in your case. One thing to keep
> > in mind here is that, since this is cpu intensive, optimizations might
> > matter. I remember back in 2011 that I did most of my measurements
> > with a "Release" build for example. But the above tests I did were
> > with a debug build, so ... dunno.
> 
> [ snip part about another optimization by Morten Kloster in r1140247,
> which seemed not to work for Stefan. ]
> 
> Coming back to this thread from last year, just wondering if you want
> to pick this up again, Stefan (or anyone else), since a 1.15 release
> might be happening in the near future.
> 
> I think we already concluded that changing this behaviour in a patch
> release would be hard to justify. But for 1.15 it might be acceptable
> in some form.
> 
> Just wanted to bring this back up, I have no vested interest here. I'm
> still skeptical about changing the output of diff & blame in cases
> where we hit "this takes too much work", but I won't stand in the way
> if that what others consense on :-). If we want to pick a limit of the
> maximum effort, then I'd suggest 16384 (or above), but that's a purely
> personal / local suggestion, because the use case I had above would
> then not be impacted.
> 
> BTW: it's a pity that we can's say: "limit the diff effort in case of
> 'update' or 'merge', but not in case of 'diff' or 'blame'". Because in
> the latter case, the user might care more about the actual output, and
> in the former they might not (until they hit a text conflict of
> course, then they will care again ...)
> 
> It might be interesting if someone could take a more "profiling" look
> at why Stefan's example takes much more time, even with limit==8192
> (18 seconds), compared to my example (3.3 seconds). Is that purely
> because of the total size of the files / number of lines? I find that
> difference quite strange. There might be something that can be
> optimized without changing behaviour. But I'm afraid I don't have time
> to dig deeply into this.

My assumption by now is that the algorithm needs to be changed.
This would essentially amount to a complete rewrite of the diff code.

There must be a fundamental difference to how 'git diff' works,
for example, which succeeds on the large files I have tested
in a relatively short amount of time.

Even 'got diff' (from my side-project gameoftrees.org), which does not
focus on performance as a first-class f

Re: svn commit: r1899311 - /subversion/branches/1.14.x/STATUS

2022-03-31 Thread Stefan Sperling
On Thu, Mar 31, 2022 at 09:21:58AM -0400, Nathan Hartman wrote:
> On Thu, Mar 31, 2022 at 9:09 AM Nathan Hartman  
> wrote:
> > My bad. Hopefully r1899430 fixes it.
> >
> > How do I manually run backport.pl?
> 
> Let me rephrase that question: How do I manually trigger it so we
> don't have to wait for the cron job?

Related question: Why don't we run the cron job more frequently? :)


Re: Backports "bot" not running?

2022-03-27 Thread Stefan Sperling
On Sun, Mar 27, 2022 at 09:35:51AM -0400, Nathan Hartman wrote:
> On Sun, Mar 27, 2022 at 9:05 AM Mark Phippard  wrote:
> >
> > On Sun, Mar 27, 2022 at 8:50 AM Nathan Hartman  
> > wrote:
> > >
> > > On Sun, Mar 27, 2022 at 7:05 AM Mark Phippard  wrote:
> > >>
> > >> On Sun, Mar 27, 2022 at 7:00 AM Daniel Sahlberg
> > >>  wrote:
> > >> >
> > >> > Hi,
> > >> > It is due to the migration of svn-qavm to the new host as requested by 
> > >> > ASF Infra. I'll look into it right away, it has been on my todo list 
> > >> > since last week, sorry!
> > >> > /Daniel
> > >>
> > >> Thanks. We will want it to get this release process completed but in
> > >> the near term once this first batch of backports are merged I am
> > >> hoping that will make all of the tests on the branch run successfully
> > >> again.
> > >
> > >
> > > Not sure if you're referring to the buildbots here but that's the other 
> > > broken important thing. I'll try to look into it soon. I think I can get 
> > > most of the buildbots running by commenting a few lines and renaming a 
> > > file.
> >
> > I am sure it would be nice to have the buildbots running if they are
> > not, but I will run the tests locally before posting any tarballs so I
> > do not feel like I need these to do the release.
> >
> > I was only referencing our automated backport script which merges
> > approved changes. That is not happening at the moment so the branches
> > are not being updated with approved changes.
> >
> > Since the 1.14.x branch currently has test failures when I run locally
> > I was just hoping to see a clean run happen so I know we are in better
> > shape to start progressing towards a release.
> 
> Last night I ran the tests on 1.14.x with the following merged and all
> tests passed for me:
> 
> r1877310 r1883355 r1878379 r1883719 r1883722 r1884610 r1881534
> r1883838 r1883989 r1886460 r1886582 r1887641 r1890013 r1889629
> r1892470 r1892471 r1892541 r1894734 r1897449 r1898633 r1899227
> 
> So hopefully all those (or the subset that fixes the broken tests)
> will be approved and merged soon...

Anyone should feel free to merge+commit approved changes.
I have often bypassed the backport merge bot while doing RM work.
This bot is just a nice-to-have convenience and its absence should not
prevent us from making progress.

Cheers,
Stefan


Re: Questions on Release Management Process

2022-03-21 Thread Stefan Sperling
On Mon, Mar 21, 2022 at 04:46:55PM -0400, Mark Phippard wrote:
> On Mon, Mar 21, 2022 at 4:31 PM Stefan Sperling  wrote:
> > This might be a swig problem? Perhaps the version of swig and the
> > version of python don't work well together? Not sure.
> 
> HACKING implies I should use the version that the clean room
> environment installs and not what the OS installs so I ran:
> 
> ./configure --with-swig=/opt/svnrm/prefix
> 
> This could be an example where the docs are just not explicit enough
> and I was overthinking. Now that I think about it maybe the problem is
> that this is not found when I run the tests. Perhaps I should use the
> OS SWIG for building and testing? Makes me wonder how the clean room
> version is ever used though?

While the clean-room env includes swig, it does not include python.
So python comes from the system. And, as far as I understand, python
is complaining about a syntax error in the code generated by swig.

I would try to find out whether the clean-room swig expects a particular
range of python versions. Maybe you can use a corresponding version of
python in your docker image.

What might also help is upgrading swig to a newer version. If upgrading
tools in the clean-room env fixes such problems then we should upgrade them.


Re: Questions on Release Management Process

2022-03-21 Thread Stefan Sperling
 and
> let the others that sign the release verify the tests all work for
> them.

The failing test has a comment on top which explains why :)

/* XXX NOTE:
   This test will fail on most Unix-like systems when run as the
   root user, because flock() will ignore file permissions. */


> Problem 3: Python bindings tests fail
> 
> I have tried this using Debian Buster and Python 2 and Debian Bullseye
> and Python 3. The Perl and Ruby bindings are working OK. I am still
> having some trouble with JavaHL but feel like I will figure that one
> out.  I have not tried the ctypes bindings yet.
> 
> This fails using the 1.14.1 tarball as well as when I make a trial run
> using release.py
> 
> # make check-swig-py
> if [ "LD_LIBRARY_PATH" = "DYLD_LIBRARY_PATH" ]; then for d in
> /opt/svnrm/subversion-1.14.1/subversion/bindings/swig/python/libsvn_swig_py
> /opt/svnrm/subversion-1.14.1/subversion/bindings/swig/python/../../../libsvn_*;
> do if [ -n "$DYLD_LIBRARY_PATH" ]; then
> LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$d/.libs"; else
> LD_LIBRARY_PATH="$d/.libs"; fi; done; export LD_LIBRARY_PATH; fi; \
> cd /opt/svnrm/subversion-1.14.1/subversion/bindings/swig/python; \
>   /usr/bin/python
> /opt/svnrm/subversion-1.14.1/subversion/bindings/swig/python/tests/run_all.py
> Traceback (most recent call last):
>   File 
> "/opt/svnrm/subversion-1.14.1/subversion/bindings/swig/python/tests/run_all.py",
> line 23, in 
> import mergeinfo, core, client, delta, checksum, pool, fs, ra, wc,
> repository, \
>   File 
> "/opt/svnrm/subversion-1.14.1/subversion/bindings/swig/python/tests/mergeinfo.py",
> line 22, in 
> from svn import core, repos, fs
>   File 
> "/opt/svnrm/subversion-1.14.1/subversion/bindings/swig/python/svn/core.py",
> line 26, in 
> from libsvn.core import *
>   File 
> "/opt/svnrm/subversion-1.14.1/subversion/bindings/swig/python/libsvn/core.py",
> line 144
> def apr_initialize() -> "apr_status_t":
>  ^
> SyntaxError: invalid syntax
> make: *** [Makefile:944: check-swig-py] Error 1

This might be a swig problem? Perhaps the version of swig and the
version of python don't work well together? Not sure.

> Problem 4: Emails
> 
> I have no idea how to send the emails from my Apache address

Configure your mail client such that when you send mail from
your @apache.org address, it uses SMTP server mail-relay.apache.org.
${user}_apache is the username (stsp_apache in my case).
The password is the same as you are using to make commits.
The server supports STARTTLS.

> 
> Problem 5: Unknowns
> 
> Since I have not been able to dry run the entire process I am still
> unsure what else I will run into and how I would solve those problems.

I believe you are already far enough down the road that any remaining
issues will be ironed out.

It is not unusual for an RM to deal with problems like you are seeing.
Sometimes the environment changes and we have to adapt the script.
Updates to the release process documentation usually happen when a
release is made and the RM runs into issue like this. Otherwise, we
mostly leave these docs alone.

Provided you don't get stuck for too long and will be able to get help
when you need it, I don't think there is anything you should worry about.

Cheers,
Stefan


Re: multi-wc-format review

2022-02-19 Thread Stefan Kueng




On 19.02.2022 16:31, Daniel Shahaf wrote:

Stefan Kueng wrote on Sat, Feb 19, 2022 at 16:11:59 +0100:

I guess I'm a little late to this discussion, but I just upgraded today to
the svn trunk and saw the new APIs.
 From what I understand is that new clients can choose which WC format to use
during checkout/upgrade?



Yes.  By default, the oldest supported WC format (currently f31,
supported by 1.8–1.14) will be used.  See [1].

[1] 
https://mail-archives.apache.org/mod_mbox/subversion-dev/202202.mbox/%3C2069FF58-3621-414A-A3AA-28CDA210017C%40getmailspring.com%3E


If that's correct, I'd like to propose a small enhancement:

* provide an env variable that specifies the highest WC format to use.

the reason for this is that some users have multiple svn clients installed,
and when upgrading one they can set this variable to avoid that not-upgraded
clients can still access new wc's. Only when all clients are upgraded they
can remove the env variable or set it to the newest format.
I figure this would mostly affect windows users, but still I think this
would be worth to implement.


This use-case is already supported by the use of the oldest WC format by
default, isn't it?


ah, yes. If the oldest supported format is used then of course the env 
variable isn't necessary. I was under the impression that the newest 
would be used by default.


Stefan


Re: multi-wc-format review

2022-02-19 Thread Stefan Kueng
I guess I'm a little late to this discussion, but I just upgraded today 
to the svn trunk and saw the new APIs.
From what I understand is that new clients can choose which WC format 
to use during checkout/upgrade?


If that's correct, I'd like to propose a small enhancement:

* provide an env variable that specifies the highest WC format to use.

the reason for this is that some users have multiple svn clients 
installed, and when upgrading one they can set this variable to avoid 
that not-upgraded clients can still access new wc's. Only when all 
clients are upgraded they can remove the env variable or set it to the 
newest format.

I figure this would mostly affect windows users, but still I think this
would be worth to implement.


Stefan



Re: svn copy --pin-externals pins relative paths to non existing revision/path

2022-02-11 Thread Stefan Sperling
On Fri, Feb 11, 2022 at 09:22:25AM +0100, Martin Obermeir wrote:
> Hi,
> 
> When using `svn copy --pin-externals` to create a tag, svn:externals with a
> relative path get pinned a non existing path: They get pinned to the
> revision before the commit, but the path doesn't exist yet in that revision.
> I expected them to be pinned to the revision which gets created during the
> svn copy operation.
> 
> In the example below:
>   actual result: A/D - ../B/lambda@2 lambda
>   expected result: A/D - ../B/lambda@3 lambda
> 
> Please find the script to reproduce this attached. Commented excerpt:
> 
>   cd A/D/
>   svn propset svn:externals "../B/lambda lambda" . # relative path
>   svn commit -m "msg1" # creates revision 2
>   cd ../..
>   svn copy --pin-externals ${URL}/trunk ${URL}/tags/pinnedExternals -m
> "msg2" # creates revision 3
>   svn switch ^/tags/pinnedExternals
>   # gives warnings:
>   #   svn: warning: W205011: Error handling externals definition for
> 'A/D/lambda':
>   #   svn: warning: W17: URL
> 'file:///tmp/pinExternalsBug/repos/tags/pinnedExternals/A/B/lambda' at
> revision 2 doesn't exist
>   svn propget -R svn:externals
>   # gives: A/D - ../B/lambda@2 lambda
>   #   i.e. pinned to revision 2, but the path doesn't exist (created in
> revision 3)
>   #   I would expect: A/D - ../B/lambda@3 lambda
> 
> 
> Tested on Kubuntu 20.04.3 LTS and SUSE Linux Enterprise Server 15 SP3 
> (x86_64) with:
> - todays trunk (Revision: 1897960): svn, version 1.15.0-dev (under
> development) compiled Feb 11 2022, 08:01:11 on x86_64-pc-linux-gnu
> - svn, version 1.14.1 (r1886195)
> - svn, version 1.13.0 (r1867053)   compiled Mar 24 2020, 12:33:36 on
> x86_64-pc-linux-gnu
> - svn, version 1.10.6 (r1863367)
> 
> Best regards
> Martin


Hi Martin,

Thank you for this report. This is a problem which was already present in
the commit which added this feature (https://svn.apache.org/r1659395).

I am afraid it is impossible to implement a solution for the issue as
you have presented it because your desired solution has an inherent
chicken-and-egg problem:

The external definition is created by the client as part of creating the
new commit. At this time, the new revision number is not yet known.
The client only learns about the new revision number after the commit has
been created on the server. And the client must prepare properties which
contain external definitions as part of creating the commit. So there is
an inherent contradiction between the behaviour you would like to see and
the way the system works internally.

The revision number cannot even be inferred as something like HEAD+1 because
the server can process commits from multiple clients in parallel while clients
upload data, assigning a new number to whichever commit finishes first.

The best suggestion I have is to only refer to already existing source paths
in externals definitions added with your commit which creates your new tag.
This means instead of using a relative external that points inside the path
space of your new tag, use a path that points into trunk or some other branch
where the same files and directories already exist. Because any tag created
by a URL-URL copy is a copy of something that already exists somewhere else
in the repository, this should always be possible, should it not?
Once the tag has been created and the new revision number is known, you
could then make a subsequent commit which manually pins the external to a
path which now exists inside the tag's path space, if you prefer that.

Does this make sense?

The behaviour exposed by your script is certainly not ideal.
In the URL-URL copy case the client does not check whether the pinned
paths even exist!
The client could at least check whether the path is is pinning does in
fact exist. It could either error out if the path cannot be found, or
simply leave the external alone and avoid pinning it.

Verifying externals in general is not an easy problem. If an external
definition is wrong or no longer working due to circumstances such as
URLs having become unusable, SVN will error out when trying to use the
external. There is little we can do about that, as it is somewhat
inherent in the design of the externals feature. Any external that is
valid today could become invalid tomorrow.

Regards,
Stefan


Re: Make 'svn patch' keep permissions of patched files

2022-02-10 Thread Stefan Sperling
On Thu, Feb 10, 2022 at 12:10:08AM -0600, Karl Fogel wrote:
> On 09 Feb 2022, Ruediger Pluem wrote:
> > When rebuilding my own Subversion build I stumbled across the following
> > patch that I add to my build:
> > 
> > Index: subversion/libsvn_client/patch.c
> > ===
> > --- subversion/libsvn_client/patch.c(revision 1897905)
> > +++ subversion/libsvn_client/patch.c(working copy)
> > @@ -3246,6 +3246,15 @@ install_patched_target(patch_target_t *target,
> > con
> > target->content->eol_style ==
> >   svn_subst_eol_style_native);
> > 
> > +  /* Make sure the patched file has the same permissions
> > the
> > +   * original file, but only if it does not get added.
> > +   */
> > +  if (!target->added)
> > +{
> > +  SVN_ERR(svn_io_copy_perms(
> > +target->local_abspath,
> > target->patched_path, pool));
> > +}
> > +
> >   SVN_ERR(svn_subst_copy_and_translate4(
> > target->patched_path,
> > target->move_target_abspath
> > 
> > It ensures that files that get modified (not added) during svn patch
> > keep their permissions.  Can this be added to trunk?
> 
> I'm curious: what is the case that causes the patched target file to have
> different permissions than it had before the patch?  (Or am I
> misunderstanding what you're saying?)

Possibly related to the fact that the patched target is prepared inside
a temporary file, which has 0600 permissions by default since it gets
created by mkstemp(3) (see https://svn.apache.org/r880338).

This temp file is then renamed on top of the file in the working copy and
file mode bits will be copied 1:1. We have had similar reports before, where
files ended up being readable only by the user who was using the working
copy, and where this interfered with some other use of such files.

> If you feel like writing a regression test for this case, then I could
> probably answer my question by looking at the test's code (but I realize you
> might not have time to do that).

I agree that having a test case to cover this behaviour would be good.
A quick glance over subversion/tests/cmdline/patch.py suggests that
this behaviour is not covered by our tests yet? That would be unfortunate.

Ruediger, you may not be aware that our /subversion subtree of the ASF
SVN repository is open for all ASF committers. You should already have
write access. So once we are done discussing this change, you will be
able to add this change to trunk yourself.

Cheers,
Stefan


Re: Streamlining Subversion patch releases

2022-02-09 Thread Stefan Sperling
On Wed, Feb 09, 2022 at 08:01:26AM -0500, Mark Phippard wrote:
> Anyway, my feeling has been that one of the blockers to being RM is
> motivation. My feeling has been that it is a fair amount of work that
> might not go anywhere because we do not have enough interest in
> reviewing and signing the release. So why put in the effort to do this
> when the votes are not going to happen?

It is a bit of a chicken and egg problem. We have never had a problem
getting the release out the door once an RM did step up. Enough people
always joined in to help once the ball got rolling.

> We have also always had what I
> thought was a peculiar policy that the RM's votes did not "count". So
> often the most motivated member of our community steps forward to do
> the RM work, and now we have lost the one sure thing vote we would
> have had for the release.

We have already made the RM's vote count during our most recent releases,
as far as I remember. At least I did it that way to expedite the process.
The ASF requires release signatures by at least 3 PMC members. Any further
restrictions are self-imposed and can be avoided if needed.


Re: Streamlining Subversion patch releases

2022-02-09 Thread Stefan Sperling
On Wed, Feb 09, 2022 at 07:23:55AM -0500, Mark Phippard wrote:
> 2. We need a RM to produce the release. Only a handful of people have
> done this and I am not one of them so I cannot comment on how hard
> this is. It does feel like this entire process could be completely
> automated though. As in, someone creates a tag or pushes a button in
> Jenkins and the rest just happens.

I think our main problem is that not enough people have gone through
this process, and it may seem more intimidating than it should be.
It is not actually a lot of work. The process is well documented and
already automated for the most part. The RM mostly runs release.py in
a series of steps, by following our docs.

Anyone on the PMC can do this. If you've made a commit to the project or
to the website, have reviewed fixes in STATUS and have communicated with
people in this project via mailing list and IRC, then you already know
everything you need to know to become the RM!

The main burden is to guess a future date for the release and then keep
track of our progress until the release is done. The RM should ensure we
don't miss the prospected date by a large margin. This requires constant
attention to the project for a while. If you cannot afford to spend roughly
half an hour per day on it for a week or two, you don't have enough time.

During this time, the RM mostly keeps track of things in STATUS and keeps
poking people to help with reviewing, testing, and signing.
The actual releasing is done by doing a few SVN commits, again via release.py.
And the RM writes release announcements, which is also automated by release.py.

There is a one-time effort to ensure that sending mail via your apache.org
account works; the ASF has an SMTP relay you can use for this purpose and
this needs to be configured in your mail client.

And you need a Linux system, which as we know is a show-stopper for some
of our developers. Anyone who wants to RM on Windows would need to port
the release.py process over first, which is probably a good chunk of work.
I would recommend setting up a Linux VM instead.


Re: A strong WTF on compiling out plaintext password support by default?!

2022-01-20 Thread Stefan Sperling
On Wed, Jan 19, 2022 at 08:08:06PM -0600, Karl Fogel wrote:
> ('svn authn' could also support a '--remove' flag to clear out the authn
> cache for a given repository.)

You may have missed that we have added the 'svn auth' command while
you were not looking :)

Removing cached creds can already be done with 'svn auth --remove'.

> I suggest the command name 'authn' rather than 'auth' in order to keep
> "authentication" (authn) distinct from "authorization" (authz).

Having both 'svn authn' and 'svn auth' commands would be confusing.
The names are too close and the proposed functionalty overlaps.
So I would prefer if the existing 'svn auth' command was extended.

I like your proposal.

Cheers,
Stefan


Re: svn commit: r1896877 - /subversion/trunk/subversion/svnadmin/svnadmin.c

2022-01-11 Thread Stefan Sperling
On Tue, Jan 11, 2022 at 12:22:38AM -0500, Nathan Hartman wrote:
> On Mon, Jan 10, 2022 at 6:44 AM  wrote:
> >
> > Author: stsp
> > Date: Mon Jan 10 11:44:46 2022
> > New Revision: 1896877
> >
> > URL: http://svn.apache.org/viewvc?rev=1896877=rev
> > Log:
> > Fix misleading -r option documentation for some svnadmin subcommands.
> >
> > The commands delrevprop, lstxns, rev-size, and setrevprop accept only
> > a single revision number. However, the -r option help text implied
> > that a revision range was accepted. Correct the -r option help text
> > of aforementioned commands.
> 
> The log message contains an error: the corrected commands are:
> delrevprop, rev-size, setlog, setrevprop
> 
> The help text of 'svnadmin lstxns' is correct and is unchanged.
> 
> Cheers,
> Nathan
> 

Thank you for pointing this out! Fixed now by replacing all occurrences
of lstxns with setlog.


Re: Fwd: Can't create temporary file from template ... No such file or directory

2021-12-06 Thread Stefan Sperling
On Sun, Dec 05, 2021 at 11:21:48PM -0500, Nathan Hartman wrote:
> On Sun, Dec 5, 2021 at 12:55 PM James McCoy  wrote:
> >
> > On Sun, Dec 05, 2021 at 12:41:30PM -0500, Nathan Hartman wrote:
> > > It's not dumping core on mine. I would like to debug this properly but
> > > am running into various nonsense. For example, I haven't figured out
> > > how to run it under gdb since svn is being called through the libtool
> > > script;
> >
> > "libtool exec gdb ./subversion/svn ..." will run svn with the
> > environment setup properly.
> 
> Perfect. That's the clue I needed. Thanks, James.
> 
> At diff_dir_added() at libsvn_client/conflicts.c:7570 (in current
> trunk, r1895609) we are attempting to concat some path components
> with:
> 
>   copyfrom_url = apr_pstrcat(scratch_pool, b->repos_root_url, "/",
>  right_source->repos_relpath, SVN_VA_NULL);
> 
> However, right_source->repos_relpath is an empty string, so we end up
> with b->repos_root_url plus a trailing slash, causing the path to
> become non-canonical.
> 
> The fix might be as simple as checking for that and not calling
> apr_pstrcat(), but I would like to dig a little more before jumping to
> conclusions.

Thanks, Nathan. I see this is in the conflict resolver so this is probably
code I have written. But I won't snag this away from you. And if you need
any help, just let me know. (I have not yet looked into details; I suspect
that right_source might be bogus or simply invalid?)

Cheers,
Stefan


next planned release

2021-11-02 Thread Stefan Küng
Hi,

I was wondering if there are any plans to make a new release of Subversion?
I'm asking because until now I made new TSVN releases in sync with
Subversion. But now with Win11 out I'd like to make a new release so users
can make use of some new features which are helpful for Win11.

It's been more than a year now since the 1.14 release, so if 1.15 is maybe
due soon I'll wait with the TSVN release. If not I'll have to make an
"out-of-sync" release.

Stefan


Re: A strong WTF on compiling out plaintext password support by default?!

2021-10-03 Thread Stefan Sperling
On Sun, Oct 03, 2021 at 12:38:48PM +0200, Daniel Sahlberg wrote:
> * Create an svn auth add command. This option has the advantage that
>   one person has expressed interest to invest time to write the code.

> Based on the reasoning above I'm proposing:
> - Adding an svn auth add command that more or less does what the scripts
> are already doing.

I still support this idea, but please don't wait for me to implement it.
I have too many other commitments for the time being.

Thank you for writing this summary. From my point of view it captures
the current situation well, and it is very useful for us to have all the
important points listed in one message.

Cheers,
Stefan


Re: A two-part vision for Subversion and large binary objects.

2021-08-28 Thread Stefan Sperling
On Fri, Aug 27, 2021 at 06:01:21PM +0200, Stefan Sperling wrote:
> Consider an asymmetric home internet connection with more download
> than upload capacity. The server may well be able to send the
> original full text much faster than the client could upload it.
> So with a sufficiently small delta the overall turnaround time might
> well be much shorter than letting the client upload a full-text copy.
> 
> In cases where the server is hosted behind such a link we will lose time.
> But I would expect this case to be less common than the former.

After some more thought on this I realize the main focus of this effort
are files which are difficult to deltify. In that case my above
consideration is likely going to be irrelevant in practice.

And I see that Evgeny has already figured out how to make the client
send a full text during commit. Great :)


Re: A two-part vision for Subversion and large binary objects.

2021-08-27 Thread Stefan Sperling
On Fri, Aug 27, 2021 at 06:42:34PM +0300, Evgeny Kotkov wrote:
> Mark Phippard  writes:
> 
> > Suppose I am using this feature for binaries, which I think is the
> > main use case. Using whatever tool I produce a modified version of my
> > binary file. At this point in time, there is nothing in the text-base
> > because I have not done any SVN operations other than checkout.  Since
> > it is a binary, other than maybe running status my next likely command
> > is going to be commit. Are you saying that during the processing of
> > this commit the client will first fetch the original text base from
> > the server before doing the commit? That seems sub-optimal.  Perhaps
> > you are only doing this for diff or revert which would make perfect
> > sense?
> 
> The current state is that the commit is going to fetch the original text-base
> before sending the delta.

Consider an asymmetric home internet connection with more download
than upload capacity. The server may well be able to send the
original full text much faster than the client could upload it.
So with a sufficiently small delta the overall turnaround time might
well be much shorter than letting the client upload a full-text copy.

In cases where the server is hosted behind such a link we will lose time.
But I would expect this case to be less common than the former.


Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-27 Thread Stefan Sperling
On Fri, Aug 27, 2021 at 11:38:57AM +, Daniel Shahaf wrote:
> The solution to "credentials can't be used to commit with" is not
> *necessarily* "cache a password for a different username".  It could
> also be that the authz file has a business logic error («r» rather than
> «rw»), or that the server admin temporarily disabled all writes to the
> repository, or that the working copy should be switched from a tag back
> to trunk, and so on.
> 
> The specific case of one username with two passwords can be implemented
> with httpd modules, I believe.  I don't know if anyone does that in
> mod_dav_svn specifically, but I know it's used in other contexts.

Indeed. The dynamic nature of auth/authz settings and the complexities
of HTTPD authentication features makes this difficult to solve in
general with a client-side only implementation.

I don't think making any server-side changes to support a potential
'svn auth add' command would be worth it. Making server-side changes
would push this idea way too far.


Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-26 Thread Stefan Sperling
On Thu, Aug 26, 2021 at 04:08:34PM -0400, Nathan Hartman wrote:
> On Thu, Aug 26, 2021 at 6:30 AM Stefan Sperling  wrote:
> > One consequence is that when Alice mistypes the --username option, or
> > mistypes the username or password at the prompt, invalid credentials will
> > be cached. Which should make any regular SVN operation fail and ask for
> > credentials again. I don't think this would be a problem, apart from the
> > possibility that invalid plaintext credentials would not be overwritten
> > by SVN binaries compiled without support for writing plaintext passwords
> > during regular operation.
> 
> That could be mitigated by providing a "svn auth remove" that deletes
> a cached credential.

Yes, removing the credential would also unblock the situation, as would
updating the credential.

Thankfully, svn auth --remove already exists :)

auth: Manage cached authentication credentials.
usage: 1. svn auth [PATTERN ...]
   2. svn auth --remove PATTERN [PATTERN ...]



Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-26 Thread Stefan Sperling
On Thu, Aug 26, 2021 at 04:17:16PM +0200, Daniel Sahlberg wrote:
> Den tors 26 aug. 2021 kl 16:10 skrev Stefan Sperling :
> 
> > On Thu, Aug 26, 2021 at 02:41:44PM +0200, Johan Corveleyn wrote:
> > > I get the feeling I'm missing something, but I still don't understand
> > > what authz has to do with the problem at hand here (i.e. detecting
> > > expired passwords so we can ask the user for the new one).
> >
> > The problem is that some repositories (like our own) do not require
> > any authentication in order to read data.
> >
> > Your case where 'svn ls' asks for a password is not applicable for
> > public repositories on svn.apache.org, for example. The 'svn auth add'
> > command would not get an authentication challenge by running the
> > equivalent of what 'svn ls' is doing. We do not have a way to trigger
> > the challenge without modifying the repository somehow.
> >
> 
> Is it possible to have the client "throw" the username/password at the
> server even if the server doesn't issue a challenge? Would the server
> validate the username/password (even though authz would allow anonymous
> access)?
> 
> /Daniel Sahlberg

Unfortunately, it is not. There are many authentication schemes and
at least two protocols to consider (HTTP + svn).
Some authentication schemes even require data that is generated on the
server when it sends the authentication request, such as a nonce.


Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-26 Thread Stefan Sperling
On Thu, Aug 26, 2021 at 12:15:39PM +, Daniel Shahaf wrote:
> Stefan Sperling wrote on Thu, 26 Aug 2021 10:30 +00:00:
> > And while we are considering read-only vs. read-write access:
> > Plaintext passwords or not, in my contrived scenario Eve could always
> > trick Alice into using a different user account by caching a set of
> > valid credentials which Eve knows. Apart from not caching credentials
> > at all I don't see a way to prevent this.
> 
> That scenario is called an "evil maid attack".  I don't think we should
> try to prevent it.  We are not in the business of posting guards to watch
> over unattended laptops.

The plaintext password pishing scenario also requires access to
local configuration files. We could simply declare it out of scope,
but that means we'd be ignoring users who are unhappy that plaintext
storage is even allowed. Just as they are unhappy about TortoiseSVN's
decryption shortcut in its cached password dialog (note that in this
case the windows domain password is often the same as the SVN password,
so leaving a laptop unlocked means anyone can get at domain creds).


Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-26 Thread Stefan Sperling
On Thu, Aug 26, 2021 at 02:41:44PM +0200, Johan Corveleyn wrote:
> I get the feeling I'm missing something, but I still don't understand
> what authz has to do with the problem at hand here (i.e. detecting
> expired passwords so we can ask the user for the new one).

The problem is that some repositories (like our own) do not require
any authentication in order to read data.

Your case where 'svn ls' asks for a password is not applicable for
public repositories on svn.apache.org, for example. The 'svn auth add'
command would not get an authentication challenge by running the
equivalent of what 'svn ls' is doing. We do not have a way to trigger
the challenge without modifying the repository somehow.


Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-26 Thread Stefan Sperling
On Thu, Aug 26, 2021 at 10:11:44AM +0200, Branko Čibej wrote:
> On 25.08.2021 21:01, Mark Phippard wrote:
> > On Wed, Aug 25, 2021 at 3:16 AM Johan Corveleyn  wrote:
> > 
> > > > Is there a way to test whether one has rw access without actually doing
> > > > a commit or a revprop edit?  It's possible with hooks, of course, but is
> > > > it also possible without hooks?
> > > I'm not sure I understand: why would I need to know that the cached
> > > credentials have read-write access?
> > I think it was a good question. It is hard to predict if a build
> > process just needs read access or read-write. If it needs the latter
> > it could complicate the effectiveness of a solution that goes down
> > this path. For example, imagine a scenario where the server allows
> > anonymous read access .. it will not even be possible to check
> > credentials unless you attempt a write operation.
> > 
> > I was never super excited about this change to disallow plain text
> > passwords but I figured fighting against a security issue is a losing
> > battle. I personally prefer the suggestion of making it a compile
> > option to disallow plain text passwords and have them enabled by
> > default and just turned off in the default configuration. The
> > alice/eve scenario while valid just does not concern me.
> > 
> > Solving with svn auth is a nice idea but I do not see it working
> > unless we have a way to authenticate for write access without writing
> > something.

Thanks for bringing this up, Daniel and Mark.
This is indeed an important point.

> There isn't in general, since authz can complicate matters. And there isn't
> currently, we don't have server-side support for that. I'm not even sure we
> could add a server-side method for this check, since the check for write
> access can be done entirely outside of Subversion. "svn authz write-check
> $url" sounds plausible until you consider all the various possible
> authn/authz checking combinations.

The answer might be that 'svn authz add' should simply not contact the
server to check credentials. Which means we cannot check upfront whether
the user running 'svn auth add' knows valid credentials.

I think this may still be better than the alternative where configuration
files can be tweaked to trick Alice into unknowingly saving her password
in plaintext while running regular SVN operations. Having 'svn auth' be
the only command which would write a plaintext password does provide some
protection in this scenario, regardless of whether credentials are checked
against the server before they get cached.

One consequence is that when Alice mistypes the --username option, or
mistypes the username or password at the prompt, invalid credentials will
be cached. Which should make any regular SVN operation fail and ask for
credentials again. I don't think this would be a problem, apart from the
possibility that invalid plaintext credentials would not be overwritten
by SVN binaries compiled without support for writing plaintext passwords
during regular operation.

In the absence of non-plaintext auth caches SVN would then keep asking
for a password until 'svn authz add --plaintext' is used again to store
valid credentials.

This problem also applies to any helper scripts that people might be
running right now in order to add plaintext credentials to the auth cache.

To mitigate this problem a bit, we could have 'svn auth add' ask for the
password twice and repeat the prompt in a loop in case of a mismatch.

And while we are considering read-only vs. read-write access:
Plaintext passwords or not, in my contrived scenario Eve could always
trick Alice into using a different user account by caching a set of
valid credentials which Eve knows. Apart from not caching credentials
at all I don't see a way to prevent this.


Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-24 Thread Stefan Sperling
On Tue, Aug 24, 2021 at 02:37:57AM -0700, Robby Zinchak wrote:
> Oh, hello all :)
> 
> Yeah, between this cli obstacle, problems with rapidsvn corrupting local
> repo during file moves, and svn Apache frequently corrupting server repo
> unrecoverably in large check-ins (>200mb) and requiring a nuke and reload
> from backup... it's been pretty rough year for my use cases of svn.  :'(

I am sorry to hear that you have been having much so trouble with SVN lately.
It would be interesting to learn more about repository corruption issues
and how they could be prevented, but this is unrelated to the discussion
at hand. Let's try to keep a positive outlook on things :)

> I have been using the stored credential work around (thanks for that), but
> it's hard to imagine new users finding this thread to know about
> undocumented workaround.

I agree that a script is unlikely to get as the same level of exposure
as a built-in command would receive.

> Stefan - I recall the previous behavior being to warn the user that
> password is being stored in plaintext during entry.  That seems like it
> would somewhat mitigate the evil actor using an official svn build threat
> case.

This prompt can be disabled in the run-time configuration, which Eve
would not hesitate to take advantage of.

> The big Pandora's Box here is that if someone has shell access, they
> can probably install or alias their own binary that does even more
> nefarious things, making any official behavior moot except for systems that
> only allow specific executable hashes to run.  (And even then, a shell
> script could mimic svns output in a phishing style attack).

Sure, the story doesn't end here.
Eve will always be looking for another trick. But only if forced to!

What matters is that these are additional hoops to jump through and may
be easier to identify as something out of the ordinary. Whereas the
previous attack was trivially concealed as harmless configuration file
edits apparently made by Alice herself.


Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-24 Thread Stefan Sperling
On Tue, Aug 24, 2021 at 12:16:48PM +0200, Johan Corveleyn wrote:
> But: obviously I have disabled, in the runtime config area, the
> warning prompt that "Your password will be stored in plaintext" (I
> have disabled it system-wide, in /etc/subversion). Yes, we know this
> and we accept it. I would not like this nag screen to be forced with
> no way for an admin to disable it. Ah, I might be able to write a
> wrapper script to dev/null the warning :-). All this effort for ...
> nothing (from my perspective).
> 
> Perhaps a meta-itch here: the old behaviour has been there for 20
> years, and then we decided to change this. In the
> super-stable-mature-super-backwards-compatible part of our lifecycle.
> Not the best timing I think. Perhaps this should have rather been on
> the 2.0-wishlist ("Come up with a better pwd caching solution on
> non-Windows platforms").

OK, I see why my proposal as it is wouldn't help you.
But if we tweak it slightly, it could work?

We could make 'svn auth add' only ask for a password if no valid
password can be found in the cache. The command would first contact
the server and if it manages to authenticate it would do nothing else.

And we could avoid showing a plaintext prompt. Instead we could require
a --plaintext command line option to allow storage in plaintext.
Without --plaintext, 'svn auth add' would fail if the password cannot
be stored encrypted and general plaintext-support was not enabled at
compile time. The only interactive component presented to the user would
be a password prompt.

Failing that, I agree that the best solution would be to simply
revert the default setting of the compile-time switch back to 'yes'.

This won't make the hardline "no plaintext, please" crowd very happy.
But the middle ground, where all this can be configured at run-time
probably won't satisfy them either. I guess that is why a compile-time
switch exists in the first place.


Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-24 Thread Stefan Sperling
On Mon, Aug 23, 2021 at 02:45:44PM +0200, Johan Corveleyn wrote:
> Thanks, those are good efforts (and thanks to both Daniels for writing
> them), but I'm afraid those workarounds are not good enough. The thing
> is: this is not for me alone. This needs to be handled in buildscripts
> that can be run by around 50 developers.

One thing is unclear to me:

At some point, someone has to run an svn command to get the passward
cached on Solaris in the first place or build scripts would be failing.

How is this done? Each of the 50 developers ran 'svn checkout' or
something similar to get the password cached? Or does the password
get cached in a non-interactive, automated way?

The point of this question is to understand whether my 'svn auth add'
proposal described elsewhere in this thread would solve your problem.


Re: A strong WTF on compiling out plaintext password support by default?!

2021-08-24 Thread Stefan Sperling
On Mon, Aug 23, 2021 at 09:05:33PM +0200, Daniel Sahlberg wrote:
> Has there been any complaints about Subversion's ability to store passwords
> in plaintext?

Of course :) Years ago, before any encrypted storage was available
on Unix systems, this was a common complaint.

The FAQ entry which you improved in https://svn.apeche.org/r1887129
was previously using language which reflected the nature of complaints
that were received: "Ahhh!  I just discovered that my Subversion client
is caching passwords in plain-text on disk!  -AHHH!"

> (I tried to search the mailing list but didn't come up with
> anything, possibly because of a lack of imagination on proper keywords).
> Maybe these complaints would have gone to the different distributions?

Perhaps some of them arrived via internal support channels of the various
companies involved in SVN's development. Not all communication with users
occurs via public channels.

> For reference, here is the e-mail where Stefan Sperling mentions the change
> in OpenBSD to re-enable support for plaintext passwords in OpenBSD: [2] I
> would encourage everyone to re-read that message since it has a good
> summary of arguments (including a link to a request from a corporate
> security group to TortoiseSVN to avoid storing a password in plaintext in
> memory).
> 
> For me the route taken by OpenBSD seems reasonable:
> - Enable plaintext passwords in the compile time defaults
> - Disable plaintext passwords in the default runtime configuration
> - Let the users re-enable it in their configuration if they want to
> 
> Pros:
> * It would not change the default behaviour.
> * It would enable users to enable plaintext passwords in configuration
> without having to recompile.
> 
> Cons:
> * Potentially some security group would argue about the possibility to
> enable plaintext passwords at all.

As someone who supported the original change to disable this feature at
compile-time, I backtracked a bit when I saw the consequences on OpenBSD
servers where unattended operation of svn in scripts is relatively common.

But I still think that Brane's concerns are warranted.
In particular (from #svn-dev IRC just now):

09:30 <@brane> the only issue i have with that is that users rarely look at 
   ~/.subversion/config and may not even be aware that the option 
   is enabled.

A potential attack would be Alice leaving her desktop unlocked,
and Eve using this chance to enable the option in Alice's config.
Eve would also clear the password cache with svn auth --remove
and disable the plaintext password prompt by setting the appropriate
option in ~/.subversion/servers.
Now Alice comes back and uses SVN as usual, unaware that her password
is being cached in plaintext, perhaps assuming it is being cached in
encrypted storage as she would reasonably expect.
When Alice leaves her desktop unlocked another time Eve collects
Alice's SVN password from plaintext storage.

The above may seem ridiculous to some (after all, Alice should not have
left her desktop defenseless!) but this is indeed a threat scenario
which is considered in some deployments. The TortoiseSVN discussion
linked earlier is based on a similar threat model where an unlocked
Windows desktop is abused to spy out passwords cached by TortoiseSVN,
which will reveal encrypted passwords in plaintext when asked to do so.

To protect Alice from this attack, writing plaintext passwords must
remain disabled during regular usage of SVN. We could provide a special
mechanism to add a plaintext password under a set of conditions:

  - The command to run is not a regularly used SVN command.
'svn auth' would be a good candidate since its sole purpose
is to inspect and manage the cached credentials store.

  - The user provides the password and authentication with this
password against the server succeeds.

  - If a non-plaintext storage backend is available, that backend
is used instead of storing the password in plaintext (but perhaps
the plaintext backend could be forced via a command line switch).

  - The user gives consent to the password being stored in plaintext
by answering 'yes' to the plaintext password prompt, in a way that
cannot be overridden by configuration files (i.e. the value of the
store-plaintext-passwords option in the servers configuration file
would be ignored by this command).

We could have 'svn auth add https://svn.example.com/svn/foo' as a
one-shot operation that satisfies the above constraints, while leaving
all other code paths compiled out which would otherwise store passwords
in plaintext. A regular 'svn update' still won't cache a password in
plaintext, unless --enable-plaintext-passwords was passed at compile-time.

Now Alice can store a plaintext password (and will be aware of its existence),
but Eve cannot fool Alice into doing so unless Subversion was compiled with
the --enable-plaintext-passwords option.

Would this work?

Regards,
Stefan


Re: svn commit: r1892471 - in /subversion/trunk/subversion: libsvn_client/conflicts.c libsvn_wc/wc_db.c tests/cmdline/merge_tree_conflict_tests.py

2021-08-23 Thread Stefan Sperling
On Sat, Aug 21, 2021 at 09:38:56PM +0200, Daniel Sahlberg wrote:
> > @@ -3028,12 +3028,12 @@ conflict_tree_get_details_local_missing(
> >deleted_basename,
> >conflict->pool);
> >details->moves = moves;
> > +  details->wc_move_targets = apr_hash_make(conflict->pool);
> >if (details->moves != NULL)
> >  {
> >apr_pool_t *iterpool;
> >int i;
> >
> > -  details->wc_move_targets = apr_hash_make(conflict->pool);
> >iterpool = svn_pool_create(scratch_pool);
> >for (i = 0; i < details->moves->nelts; i++)
> >  {
> >
> 
> I have not investigated further (ENOTIME right now) but I presume some
> other part of the code expects wc_move_targets to be NULL.

The problem is that some parts of the code try to search the now non-NULL
hash map with a NULL key because they lack checks for NULL keys.
I will commit a fix shortly.


Re: 1.14.x test failure under USE_HTTPV1=1: ra-test 13 commit_empty_last_change

2021-08-08 Thread Stefan Sperling
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


Re: [PATCH] limit diff effort to fix performance issue

2021-07-08 Thread Stefan Sperling
On Tue, Jun 08, 2021 at 05:58:14PM +0200, Johan Corveleyn wrote:
> Also: one of the last things that Morten Kloster committed in 2011 was
> another performance improvement, namely "restarting the LCS" in some
> specific cases. At the time, I didn't have enough time to review this
> properly, and had some reservations (but mainly lack of time), so I
> asked him to commit it on a branch (diff-improvements). This feature
> branch never made it to trunk. Just as another "shot", perhaps you
> could try r1140247 and see if it helps?

Unfortunately, r1140247 doesn't help in my case.
I see the new logic which was added trigger once very early on.
But it soon gets stuck in the loop forever just like the code on trunk does.

The code added in r1140247 seems unfinished and untested.
It has a bug in clear_fp() which clang warns about (see below).
Fixing that alone doesn't help my test case though.

subversion/libsvn_diff/lcs.c:264:19: warning: incompatible pointer types 
assigning to 'svn_diff__lcs_t *'
  (aka 'struct svn_diff__lcs_t *') from 'svn_diff__lcs_t **' (aka 'struct 
svn_diff__lcs_t **'); dereference with *
  [-Wincompatible-pointer-types]
lcs->next = lcs_freelist;
  ^ 
*
subversion/libsvn_diff/lcs.c:265:22: warning: incompatible pointer types 
assigning to 'svn_diff__lcs_t **'
  (aka 'struct svn_diff__lcs_t **') from 'svn_diff__lcs_t *' (aka 'struct 
svn_diff__lcs_t *'); take the address with &
  [-Wincompatible-pointer-types]
lcs_freelist = lcs;
 ^ ~~~
   &
2 warnings generated.


Re: Add a few items to svn:ignore in https://svn.apache.org/repos/asf/subversion/trunk

2021-06-23 Thread Stefan Sperling
On Wed, Jun 23, 2021 at 09:56:05PM +0200, Daniel Sahlberg wrote:
> Hi,
> 
> I'm rebuilding TortoiseSVN to test some of the new performance enhancements
> in /trunk. The TSVN source tree fetch the sources of Subversion using
> svn:externals which is kind of neat. The build process creates a few build
> folders: [release|debug]_[win32|x64][_static].
> 
> I would like to add these to the svn:ignore property on /trunk, this way it
> would be easier to see if I have made any local modification to the
> Subversion sources:
> 
> [[[
> C:\Devel\tortoisesvn\ext>svn diff --depth immediates Subversion
> Index: Subversion
> ===
> --- Subversion  (revision 1891001)
> +++ Subversion  (working copy)
> 
> Property changes on: Subversion
> ___
> Modified: svn:ignore
> ## -29,7 +29,13 ##
>  apr-util
>  apr-iconv
>  Release
> +release_win32
> +release_win32_static
> +release_x64
>  Debug
> +debug_win32
> +debug_win32_static
> +debug_x64
>  ipch
>  subversion_msvc.dsw
>  subversion_msvc.ncb
> 
> C:\Devel\tortoisesvn\ext>
> ]]]
> 
> As a side note there are already a bunch of ignores for build directories,
> Release/Debug just above and there is also a "db4-win32" so I think there
> is a precedence to add these. Any opinions?

I don't see any reason against adding these. +1

Cheers,
Stefan


Re: [PATCH] limit diff effort to fix performance issue

2021-06-08 Thread Stefan Sperling
On Tue, Jun 08, 2021 at 02:57:34PM +0200, Johan Corveleyn wrote:
> Okay, I focused on the first revision causing the annotate to differ,
> and with some debug logging:
> - p went up to 139
> - length[0]=1942, length[1]=1817
> 
> Now, 1942 lines on the left and 1817 on the right doesn't seem all
> that many (that's what remains after prefix-suffix stripping). I see
> no reason 'svn diff' shouldn't be able to calculate a minimal diff for
> those sizes in a reasonable amount of time. So if p can go up to such
> a "cost" for such "reasonable" lenghts, I imagine we should put the
> actual limit much higher. I suppose we can just as well set "min_cost"
> to 1024 or even higher. 64 is way too low.

Thanks!
I have set the value back to at least 1024 with this new version of patch.

> BTW, the actual revision (and proper diff) here was not really
> unreadable. It had 86 hunks. The log message was something like
> "refactor contact parameters", and the change was removing 4 lines and
> replacing them with 1 line, throughout an entire section. If the
> lcs-limit gets hit, the entire "remaining part" (which normally would
> have been some more small hunks) becomes one giant remove+add of 1000
> lines or so (i.e. quite useless for a diff that a human might want to
> review).

Indeed, we want to avoid such changes if possible, and only punish
the cases which have problematic run-time behaviour.

> Finally, a couple of small remarks related to the patch itself:
> [[[
> +  int max_cost;
> +  /* The minimum cost 'p' (# moves away from k=0) we are willing to invest. 
> */
> +  const int min_cost = 64;
> ]]]
> 
> I get confused by the naming of the variables here (unless we use
> min_max_cost or something). Perhaps something like 'cost_limit' and
> 'min_cost_limit' or something like that?

Sure, done.

> [[[
> +  max_cost = shift_sqrt(length[0] + length[1] / 2);
> ]]]
> 
> Is that correct WRT operator precedence? It looks like you want to
> take the sqrt of the average length, which would be (length[0] +
> length[1]) / 2. Or maybe I misunderstand. I'm not really sure why this
> is used as an estimate for the "acceptable cost" / max_cost. But then
> again, it's been a while since I read the details of Myers algorithm.

Good catch. In my test case it doesn't make a difference. The limit ends
up begin 2048 in either case. But you're right, the code was incorrect.

[[[
* subversion/libsvn_diff/lcs.c
  (shift_sqrt): New helper function. Borrowed from Game of Trees' diff.
  (svn_diff__lcs): Limit the effort spent on finding an optimal diff to
   avoid spinning on the CPU for a long time (30 minutes or more) for
   some input files. Large XML documents were known to trigger this.
]]

Index: subversion/libsvn_diff/lcs.c
===
--- subversion/libsvn_diff/lcs.c(revision 1890390)
+++ subversion/libsvn_diff/lcs.c(working copy)
@@ -227,7 +227,18 @@ prepend_lcs(svn_diff__lcs_t *lcs, apr_off_t lines,
   return new_lcs;
 }
 
+/* Integer square root approximation */
+static int
+shift_sqrt(apr_off_t val)
+{
+  int i;
 
+  for (i = 1; val > 0; val >>= 2)
+i <<= 1;
+
+  return i;
+}
+
 svn_diff__lcs_t *
 svn_diff__lcs(svn_diff__position_t *position_list1, /* pointer to tail (ring) 
*/
   svn_diff__position_t *position_list2, /* pointer to tail (ring) 
*/
@@ -245,7 +256,10 @@ svn_diff__lcs(svn_diff__position_t *position_list1
   svn_diff__snake_t *fp;
   apr_off_t d;
   apr_off_t k;
-  apr_off_t p = 0;
+  apr_off_t p = 0; /* # moves away from k=0 */
+  /* The minimum cost 'p' (# moves away from k=0) we are willing to invest. */
+  const int min_cost_limit = 1024;
+  int max_cost_limit;
   svn_diff__lcs_t *lcs, *lcs_freelist = NULL;
 
   svn_diff__position_t sentinel_position[2];
@@ -337,6 +351,24 @@ svn_diff__lcs(svn_diff__position_t *position_list1
   fp[d - 1].position[0] = sentinel_position[0].next;
   fp[d - 1].position[1] = _position[1];
 
+  /* Limit the effort spent looking for a diff snake. If files have
+   * very few lines in common, the effort spent to find nice diff
+   * snakes is just not worth it, the diff result will still be
+   * essentially minus everything on the left, plus everything on
+   * the right, with a few useless matches here and there.
+   *
+   * Very large files which have a lot of common lines interleaved with
+   * changed lines (such as humongous auto-generated XML documents) could
+   * easily keep us busy here for a very long time, in the order of hours.
+   * In such cases we abort the algorithm before it is finished and use
+   * the most recently computed intermediate result. The diff will not be
+   * pretty but a quick suboptimal result is better than a perfect result
+   * which takes hours to compute.
+   */
+  max_cost_limit = shift_sqrt((length[0] + length[1]) / 2);
+  if (max_cost_limit < min_cost_limit)
+max_cost_limit = min_cost_limit;
+
   p = 0;
   do
 {
@@ -353,7 +385,7 @@ 

Re: [PATCH] limit diff effort to fix performance issue

2021-06-08 Thread Stefan Sperling
On Tue, Jun 08, 2021 at 01:45:00AM -0400, Nathan Hartman wrote:
> In order to do some testing, I needed some test data that reproduces
> the issue; since stsp can't share the customer's 100MB XML file, and
> we'd probably want other inputs or sizes anyway, I wrote a program
> that attempts to generate such a thing. I'm attaching that program...
> 
> To build, rename to .c extension and, e.g.,
> $ gcc gen_diff_test_data.c -o gen_diff_test_data
> 
> To run it, provide two parameters:
> 
> The first is a 'seed' value like you'd provide to a pseudo random
> number generator at init time.
> 
> The second is a 'length' parameter that says how long (approximately)
> you want the output data to be. (The program nearly always overshoots
> this by a small amount.)
> 
> Rather than using the system's pseudo random number generator, this
> program includes its own implementation to ensure that users on any
> system can get the same results when using the same parameters. So if
> different people want to test with the same sets of input, you only
> have to share 2 numbers, rather than send each other files >100MB of
> useless junk.
> 
> Example: Generate two files of approx 100 MB, containing lots of
> differences and diff them:
> 
> $ gen_diff_test_data 98 100m > one.txt
> $ gen_diff_test_data 99 100m > two.txt
> $ time diff one.txt two.txt > /dev/null
> 
> With the above parameters, it takes my system's diff about 50 seconds
> to come up with something that looks reasonable at a glance; svn's
> diff has been crunching away for a while now...

Thank you Nathan, this is incredibly useful!

Would you consider committing this tool to our repository, e.g. somewhere
within the tools/dev/ subtree?

Thanks,
Stefan


Re: [PATCH] limit diff effort to fix performance issue

2021-06-06 Thread Stefan Sperling
On Sat, Jun 05, 2021 at 08:56:24PM +0200, Johan Corveleyn wrote:
> Hmm, I tried this patch with my "annotate large XML file with deep
> history test", but the result isn't the same as with 1.14. I'd have to
> investigate a bit more to find out where it took another turn, and
> what that diff looks like (and whether or not the "other diff" is a
> problem for me).

It would be good to know how many iterations your file needs to produce
a diff without a limit. You can use something like SVN_DBG(("p=%d\n", p))
to print this number.

Could it help to use the new heuristics only in cases where our existing
performance tricks fail? I.e. enforce a limit only when the amount of
common tokens between the files is low, and the amount of common prefix/suffix
lines relative to the total number of lines is low?


Re: [PATCH] limit diff effort to fix performance issue

2021-06-03 Thread Stefan Sperling
On Wed, Jun 02, 2021 at 02:20:12PM +0200, Stefan Sperling wrote:
> The patch below implements an effort limit for Subversion's LCS diff.

Here is an updated version of this patch. I took a closer look at
other diff implementations again, and realized that they don't use
a fixed limit but one which is scaled to the size of the Myers graph.
With a square root applied to keep the effort within certain bounds
as the files being compared become larger.

Compared to the previous patch, I have added comments, and renamed
'max_effort' to 'max_cost' to better align with existing documentation
of the algorithm in existing comments.

We already maintain a variable 'p' which represents the cost of
the algorithm. So instead of maintaining a separate counter I am
now setting an upper bound on 'p'.

With this new patch the limit ends up being 2048 for the files I have.
For regular files in a working copy of SVN trunk, the loop typically
needs less than 10 iterations.

For reference, run-times of 'svn diff' on my test files with various
limits, including no limit:

limit = 1024
0m06.82s real 0m05.40s user 0m01.41s system
limit = 2048
0m08.15s real 0m06.55s user 0m01.56s system
limit = 4096
0m11.10s real 0m09.59s user 0m01.52s system
limit = 8192
0m18.40s real 0m16.57s user 0m01.57s system
limit = 65536
7m55.40s real 7m53.81s user 0m01.58s system
Full run without limit:
  163m31.49s real   163m16.77s user 0m02.37s system

Git/libxdiff somehow manages to produce a smaller collection of larger
chunks on the problematic files, rather than many small chunks with
one giant chunk at the end which we end up produing when we bail out
of the loop.
I have made some attempts to produce a more readable diff result in
case the limit is reached but I haven't been successful. Ideas welcome.
Still, I don't think it makes sense to invest a lot of time into
optimizing output in this case, under the assumption that nobody is
going to be carefully reading affected diffs anyway.

[[[
* subversion/libsvn_diff/lcs.c
  (shift_sqrt): New helper function. Borrowed from Game of Trees' diff.
  (svn_diff__lcs): Limit the effort spent on finding an optimal diff to
   avoid spinning on the CPU for a long time (30 minutes or more) for
   some input files. Large XML documents were known to trigger this.
]]

Index: subversion/libsvn_diff/lcs.c
===
--- subversion/libsvn_diff/lcs.c(revision 1890390)
+++ subversion/libsvn_diff/lcs.c(working copy)
@@ -227,7 +227,18 @@ prepend_lcs(svn_diff__lcs_t *lcs, apr_off_t lines,
   return new_lcs;
 }
 
+/* Integer square root approximation */
+static int
+shift_sqrt(apr_off_t val)
+{
+  int i;
 
+  for (i = 1; val > 0; val >>= 2)
+i <<= 1;
+
+  return i;
+}
+
 svn_diff__lcs_t *
 svn_diff__lcs(svn_diff__position_t *position_list1, /* pointer to tail (ring) 
*/
   svn_diff__position_t *position_list2, /* pointer to tail (ring) 
*/
@@ -247,6 +258,9 @@ svn_diff__lcs(svn_diff__position_t *position_list1
   apr_off_t k;
   apr_off_t p = 0;
   svn_diff__lcs_t *lcs, *lcs_freelist = NULL;
+  int max_cost;
+  /* The minimum cost 'p' (# moves away from k=0) we are willing to invest. */
+  const int min_cost = 64;
 
   svn_diff__position_t sentinel_position[2];
 
@@ -337,6 +351,24 @@ svn_diff__lcs(svn_diff__position_t *position_list1
   fp[d - 1].position[0] = sentinel_position[0].next;
   fp[d - 1].position[1] = _position[1];
 
+  /* Limit the effort spent looking for a diff snake. If files have
+   * very few lines in common, the effort spent to find nice diff
+   * snakes is just not worth it, the diff result will still be
+   * essentially minus everything on the left, plus everything on
+   * the right, with a few useless matches here and there.
+   *
+   * Very large files which have a lot of common lines interleaved with
+   * changed lines (such as humongous auto-generated XML documents) could
+   * easily keep us busy here for a very long time, in the order of hours.
+   * In such cases we abort the algorithm before it is finished and use
+   * the most recently computed intermediate result. The diff will not be
+   * pretty but a quick suboptimal result is better than a perfect result
+   * which takes hours to compute.
+   */
+  max_cost = shift_sqrt(length[0] + length[1] / 2);
+  if (max_cost < min_cost)
+max_cost = min_cost;
+
   p = 0;
   do
 {
@@ -353,7 +385,7 @@ svn_diff__lcs(svn_diff__position_t *position_list1
 
   p++;
 }
-  while (fp[0].position[1] != _position[1]);
+  while (fp[0].position[1] != _position[1] && p < max_cost);
 
   if (suffix_lines)
 lcs->next = prepend_lcs(fp[0].lcs, suffix_lines,



[PATCH] limit diff effort to fix performance issue

2021-06-02 Thread Stefan Sperling
I've heard of several instances where users complain that 'svn update'
takes an extraordinary amount of time to run (in terms of hours).

At elego we have received files which reproduce such behaviour.
These files are XML files that are almost 100MB in size. While versioning
such data is not the primary use case for Subversion, this should not get
in the way of people getting work done. So I would like to get this fixed.

The root of the performance problem is located in Subversion's diff code.

The files contain in the order of 1.200.000 lines of XML text. I cannot
share the full files, unfortunately. Here's one diff chunk from a diff
between two such files, just to give you an idea about the type of content
which the diff algorithm is trying to split into chunks. Simply imagine that
such content goes on for more than a million lines and you have a mental
image of what the file generally looks like:

  
  
 CounterDbCounter 
 ARRAY
-163  
+193^M
 
 
 
RamEccDoubleBitError_CounterDbCounter  
 

Generating a diff between two versions of such files with 'svn diff' takes
more than 40 minutes on my machine. I stopped it at that point. This is
obviously taking too long and we don't have to care about exactly how
long it will take to eventually run to completion.

As a reference, Git manages to produce a diff in about 13 seconds:

$ time git diff 01.arxml 02.arxml > /dev/null
0m13.39s real 0m13.04s user 0m00.36s system

Without my patch, the svn update/diff processes are spinning in the
following loop:

[[[
  do
{
  /* For k < 0, insertions are free */
  for (k = (d < 0 ? d : 0) - p; k < 0; k++)
{
  svn_diff__snake(fp + k, token_counts, _freelist, pool);
}
  /* for k > 0, deletions are free */
  for (k = (d > 0 ? d : 0) + p; k >= 0; k--)
{
  svn_diff__snake(fp + k, token_counts, _freelist, pool);
}

  p++;
}
  while (fp[0].position[1] != _position[1]);
]]]

What this loop is trying to do is to find an optimal "diff snake",
a partial traversal of the Myers graph which builds up step-by-step
towards some optimal traversal of the entire graph.

In the Myers diff algorithm, the problem of finding an optimal graph
traversal doesn't have a single answer. There can be several valid
traversals, resulting in differences in the generated diff.
An extreme example is a traversal which produces a patch that first
removes all lines from file A, and then adds all lines from file B.
This isn't a useful diff chunk for humans to read, but it is a valid
diff and will work when applied with a patch program.
Finding this particular traversal is also trivial and very fast :-)

The search for an optimal traversal which will result in a human-readable
diff can take time, and this is exactly where Subversion is spending too
much effort when diffing large XML files.

Other diff implementations limit the effort spent by aborting after a
certain number of search iterations, and then simply use the valid
traversal which was most recently discovered. The libxdiff code used by Git
does this, as does the diff implementation which Neels Hofmyer wrote for
Game of Trees (see https://git.gameoftrees.org/gitweb/?p=diff.git;a=summary)

The patch below implements an effort limit for Subversion's LCS diff.
Diffing the offending XML files in a Subversion working copy becomes
fast enough to be usable:

$ time svn diff > /dev/null
0m06.82s real 0m05.40s user 0m01.41s system

The resulting diff doesn't look as pretty as Git's diff output, but I don't
think this will matter in practice.

My only concern is that some corner-case diffs that are currently readable
could become less readable. If such cases are found, we could easily address
the problem by bumping the limit. Note: Any such diffs would still be *valid*.

The test suite is passing, which implies that trivial diffs aren't affected
by this change. I expect that most, if not all, diffs which people actually
want to read will remain unaffected. But I cannot tell with 100% certainty.

Before anyone asks, without evidence that it is really needed I don't think
adding a config option to set this limit is warranted. Other implementations
are also using hard-coded limits. We could add an environment variable to
allow for experimentation, perhaps. In any case, I would prefer to make
such tweaks in follow-up patches and keep this initial fix simple so it
can be backported easily.

[[[
* subversion/libsvn_diff/lcs.c
  (svn_diff__lcs): Limit the effort spent on finding an optimal diff to
   avoid 

Re: svn commit: r1890318 - /subversion/site/staging/faq.html

2021-05-30 Thread Stefan Sperling
On Sun, May 30, 2021 at 09:30:58PM +0100, Julian Foad wrote:
> Stefan Sperling wrote:
> > Nathan Hartman wrote:
> >>> +  http://libera.chat;>Matrix or
> >> 
> >> How does this URL work? Does it have to be opened from within a Matrix app?
> >> 
> >> (My browser doesn't know what to do with it, but I don't have a Matrix app
> >> installed.)
> >
> > Yes, I believe so. I don't use matrix myself.
> > Julian has tested it and says it is working.
> 
> No no: I confirmed the matrix bridge is working, not that URL. That URL looks 
> completely bogus. Correct URLs are
> 
> https://matrix.to/#/#svn:libera.chat
> https://matrix.to/#/#svn-dev:libera.chat
> 
> - Julian

Ok, thank you for the correction. Fixed on the live site now.

Sorry about the confusion. I obtained the link from Mastodon where
it was shown in a format which didn't include the matrix.to part.
Perhaps that missing part was implied and I misunderstood this, due
to my lack of familiarity with the tooling.

Cheers,
Stefan


Re: svn commit: r1890318 - /subversion/site/staging/faq.html

2021-05-30 Thread Stefan Sperling
On Sun, May 30, 2021 at 09:26:25AM -0400, Nathan Hartman wrote:
> On Sun, May 30, 2021 at 3:40 AM  wrote:
> > +  http://libera.chat;>Matrix or
> 
> 
> How does this URL work? Does it have to be opened from within a Matrix app?
> 
> (My browser doesn't know what to do with it, but I don't have a Matrix app
> installed.)

Yes, I believe so. I don't use matrix myself.
Julian has tested it and says it is working.


Re: Migrate off Freenode IRC?

2021-05-26 Thread Stefan Sperling
On Wed, May 26, 2021 at 05:29:49PM +0200, Daniel Sahlberg wrote:
> I can draft a short news item and put it on staging. I think it should be
> low key ("we have decided to move"), or does someone think we should make a
> statement ("we move because XXX is a [insert suitable invective]")?

We don't need to state a reason. I think keeping our official project
statements short and quiet is the better move in a situation like this.

> The SVN book seems to contain "freenode" only in the sample repository
> (which is a copy of the README file in
> http://svn.apache.org/repos/asf/subversion/trunk/README).
> 
> Speaking of the README. Would someone like to fix it? It is formally out of
> scope for me but I suppose it would go under obvious-fix rules (related to
> the website changes).

I would consider this as an obvious fix, yes.


Re: Migrate off Freenode IRC?

2021-05-26 Thread Stefan Sperling
On Wed, May 26, 2021 at 10:52:42AM +0200, Stefan Sperling wrote:
> On Wed, May 26, 2021 at 10:41:28AM +0200, Daniel Sahlberg wrote:
> > Considering latest development (the channel takeovers mentioned by Stefan
> > Sperling in another part of the thread), I'd suggest that we switch the
> > website. I would go for a more drastic change than Nathan's patch (which
> > looked nice just two days ago) and put libera as the official channel and
> > freenode as "in the past there were also freenode but it has been retired
> > and is no longer used by the project".
> > 
> > I could look at this tonight unless someone else beat me to it.
> 
> I have just committed a patch to the staging area which replaces links
> to freenode with links to libera.chat.
> 
> Please feel free to take a look at it and merge changes to 'publish' as
> soon as it is convenient for you to do so :)

And I agree that it would be good to mention our freenode channels
as having been retired. If you could add a statement to this effect
on top of my link changes, and/or tweak whatever else you see that
you think should be added or changed, I would appreciate it.


Re: Migrate off Freenode IRC?

2021-05-26 Thread Stefan Sperling
On Wed, May 26, 2021 at 10:41:28AM +0200, Daniel Sahlberg wrote:
> Considering latest development (the channel takeovers mentioned by Stefan
> Sperling in another part of the thread), I'd suggest that we switch the
> website. I would go for a more drastic change than Nathan's patch (which
> looked nice just two days ago) and put libera as the official channel and
> freenode as "in the past there were also freenode but it has been retired
> and is no longer used by the project".
> 
> I could look at this tonight unless someone else beat me to it.

I have just committed a patch to the staging area which replaces links
to freenode with links to libera.chat.

Please feel free to take a look at it and merge changes to 'publish' as
soon as it is convenient for you to do so :)

Meanwhile, I have also disconnected my IRC client from freenode.


Re: Migrate off Freenode IRC?

2021-05-26 Thread Stefan Sperling
On Wed, May 26, 2021 at 09:42:36AM +0200, Stefan Sperling wrote:
> On Mon, May 24, 2021 at 08:10:40PM +, Daniel Shahaf wrote:
> > I've gone ahead and moved wayita to libera.
> > 
> > I wanted to keep a copy on freenode, but forgot to change the pointer to
> > the sqlite database, and I'm not sure whether it's possible to use the
> > same set of factoids for two different IRC servers.
> 
> So far there were no objections to moving off freenode.
> Can we go ahead and mark our freenode channels as unofficial or otherwise
> close them somehow?
> 
> Be prepared for a possibility of losing control of our freenode channels.
> This morning there were channel takeovers on freenode, of channels which
> mention the competing libera network in their topic. Two examples of many:
> 
> https://www.reddit.com/r/haskell/comments/nl74hc/freenode_has_unilaterally_taken_over_haskell/
> 
> https://i.imgur.com/k5xAud4.png
> 

I've marked the channels as no longer used and moved to libera.chat
in the topic.

Arfrever had the idea of rendering libera.chat in wide unicode letters
instead of ascii, which might escape a potential freenode channel
takeover script for a while.


Re: Migrate off Freenode IRC?

2021-05-26 Thread Stefan Sperling
On Mon, May 24, 2021 at 08:10:40PM +, Daniel Shahaf wrote:
> I've gone ahead and moved wayita to libera.
> 
> I wanted to keep a copy on freenode, but forgot to change the pointer to
> the sqlite database, and I'm not sure whether it's possible to use the
> same set of factoids for two different IRC servers.

So far there were no objections to moving off freenode.
Can we go ahead and mark our freenode channels as unofficial or otherwise
close them somehow?

Be prepared for a possibility of losing control of our freenode channels.
This morning there were channel takeovers on freenode, of channels which
mention the competing libera network in their topic. Two examples of many:

https://www.reddit.com/r/haskell/comments/nl74hc/freenode_has_unilaterally_taken_over_haskell/

https://i.imgur.com/k5xAud4.png


Re: Migrate off Freenode IRC?

2021-05-23 Thread Stefan Sperling
On Fri, May 21, 2021 at 03:02:08PM +0200, Branko Čibej wrote:
> Every time I hear (or read) someone explaining how "one of my companies is
> funding this free service", I grab my wallet and run away. "Monetizing"
> community-lead and -created services has been a thing since before the
> Internet. This is probably no different.

It is going downhill faster than I expected, with policy changes like this:
https://github.com/freenode/web-7.0/pull/513/files#diff-0e382b024f696a3b7a0ff3bce24ae3166cc6f383d059c7cc61e0a3ccdeed522cL87
You are now welcome to use political, racial, ethnic, religious or
gender-related slurs on freenode. IRC trolls are going to love this.


Re: Migrate off Freenode IRC?

2021-05-21 Thread Stefan Sperling
On Thu, May 20, 2021 at 04:15:23PM +0100, Julian Foad wrote:
> For contrast:
> "The Panic Over Freenode Isn’t Justified and Its Reaction Mostly 
> Disproportionate"
> http://techrights.org/2021/05/19/freenode-and-cancel-culture/
> 
> It took me a while to find an article that covered the issue in what seems a 
> more balanced way than most, and I just want to share that. I still don't 
> know how to judge the whole thing.

This article doesn't make a lot of sense to me.

Obviously the staff who left are very, very angry about issues that
have been going on for a while.
I don't believe for a second that a dispute over a sponsorship logo
on the web site alone would explain this. 


Re: Migrate off Freenode IRC?

2021-05-20 Thread Stefan Sperling
On Thu, May 20, 2021 at 11:43:43AM +0100, Julian Foad wrote:
> Stefan Sperling wrote:
> > Keeping a matrix bridge to the current freenode channels alive means we're
> > not actually leaving this network. To support the former freenode staff it
> > makes more sense to switch IRC server *and* possibly look into a migration
> > to Matrix. I don't think these are conflicting goals.
> 
> Yes, I think those two would go nicely together, and I expect we will see
> Matrix enthusiasts setting up bridging to the replacement alongside the
> original Freenode bridge for everyone. In fact I was surprised they didn't
> blog out about that the same day; I'm sure they will soon.

OK, then let's focus on a possible Freenode -> Libera move first.

Daniel Shahaf and myself have control of #svn and #svn-dev on Libera.
We could wait a little bit and see whether the new service is indeed stable.
Based on what I'm seeing so far it looks like a several communities have
successfully migrated already. The service is runing fine at the moment.

Another channel I run (#gameoftrees) has migrated successfully. This
channel has several users, including bots, who have re-joined our
channel on the new Libera service via their own Matrix bridge.

In order to signal our move to the new IRC service, I believe we should
mark our freenode channels as 'moved to libera.chat' in their topic lines,
as soon as we have consensus in this project that we indeed want to move.

Are there any objections to this?

My main goal here is to support the former freenode staff. I don't expect
that the existing freenode service will see disruption very soon, unless
freenode collapses entirely as a result of mass-migration elsewhere.
We could also decide to stay on freenode for now to wait and see.


Re: Migrate off Freenode IRC?

2021-05-20 Thread Stefan Sperling
On Wed, May 19, 2021 at 07:23:43PM +0100, Julian Foad wrote:
> Looks like we might want to move our channels, bots, and so on?

> Libera.chat [2] has been posited as a direct replacement.
 
> However, the sane replacement these days is Matrix [1].

I understand Matrix has more modern technology, and I understand your
motivation in pushing for it. I took this opportunity to take a look.

First thing I checked was which clients are available for me to install
on OpenBSD systems. There's a plugin for pidgin and a client called gomuks.
I suppose gomuks could work. I am currently using irssi. I would now have
to run both irssi (for other IRC channels) and gomuks, or switch to gomuks
completely and rely on bridges for IRC access. There's also the server
component dendrite I could install. I don't think I would want to start
out in Matrix by running my own server, though. I've read somewhere that
Matrix servers require a lot of resources to run, but I don't know if that
is still the case.
Many more clients are listed on the matrix website but I could not easily
install those on OpenBSD without porting them first.

I could install Element on my phone, though I don't think I would ever
really follow svn/svn-dev chatter on the phone.

Compared to the cost of installing and learning to use a new client, the
cost of switching to Libera was almost zero. I could start using this new
drop-in replacement service without even restarting my IRC client, and
channel setup and nick registration were done in less than 30 minutes.

Given that there is still a non-trivial amount of regulars hanging out
in the #svn channel, the cost of switching everyone of them over is not
going to be negligible, no matter what.

Technology aside, some other thoughts:

Keeping a matrix bridge to the current freenode channels alive means we're
not actually leaving this network. To support the former freenode staff it
makes more sense to switch IRC server *and* possibly look into a migration
to Matrix. I don't think these are conflicting goals.

The freenode problem apparently happened because of for-profit investment.
I don't know the details, but as far as I understand a company was registered
and then sold to someone who is now trying to use their monetary influence in
ways that existing community members disagree with.

The new Libera network claims to be operated by an unnamed non-profit
association in Sweden. According to whois, the libera.chat domain is held
by someone in Reykjavik, Iceland. I couldn't find details, but I don't see
a reason to doubt that former freenode staff no what they are doing.
It seems like they intentionally set up a new structure to prevent their
current problem from re-occurring.

Whereas Matrix development is primarily backed by a for-profit, vector-im.
By design, and in the long term(!), this structure is vulnerable to the
same type of problem that just happened to freenode.
I understand that ad-hoc federation can help mitigate this risk, but in
the end we will always have to rely on a healthy community to operate the
service for us, whether the service uses a "modern web" ad-hoc federation
style, or the traditional federation style of IRC which requires close
coordination and bonds of trust between admins.

Cheers,
Stefan


Re: Review of task.c as of svn commit: r1888520 - in /subversion/trunk: build.conf subversion/libsvn_subr/task.c

2021-04-12 Thread Stefan Sperling
On Sun, Apr 11, 2021 at 07:55:33AM +0200, Stefan Sperling wrote:
> Just adding another thing: It looks like task.c has undefined symbols
> during linking if APR is built without support for threads:

I see that this has been fixed now. Thank you!


Re: Review of task.c as of svn commit: r1888520 - in /subversion/trunk: build.conf subversion/libsvn_subr/task.c

2021-04-10 Thread Stefan Sperling
On Thu, Apr 08, 2021 at 08:01:43PM +, Daniel Shahaf wrote:
> Good morning Stefan,
> 
> stef...@apache.org wrote on Thu, Apr 08, 2021 at 14:35:52 -:
> > Initial, single-theaded implementation of the svn_task__t API.
> 
> I've committed some minor fixes in r1888533.  Most of them are
> straightforward but please double check that all the apostrophes are
> where they should be.
> 
> Here's also a review of task.c as it stands now.

Just adding another thing: It looks like task.c has undefined symbols
during linking if APR is built without support for threads:

../../subversion/libsvn_subr/.libs/libsvn_subr-1.so.0.0: undefined reference to 
`apr_thread_exit'
../../subversion/libsvn_subr/.libs/libsvn_subr-1.so.0.0: undefined reference to 
`apr_thread_join'
../../subversion/libsvn_subr/.libs/libsvn_subr-1.so.0.0: undefined reference to 
`apr_thread_create'
https://ci.apache.org/builders/svn-bb-openbsd/builds/673/steps/Build/logs/stdio

There's probably just a missing check for the APR_HAS_THREADS macro.

This build bot is running a non-threaded build in order to catch such issues.
In reality this is likely an non-issue for most people. But as long as we
intend to support the !APR_HAS_THREADS case our code should compile in
both cases.

I don't have time right now to fix this myself, sorry.

Cheers,
Stefan


Re: svn commit: r1888446 - /subversion/trunk/subversion/include/private/svn_task.h

2021-04-08 Thread Stefan Fuhrmann




On 07.04.21 06:47, Nathan Hartman wrote:

On Tue, Apr 6, 2021 at 2:37 PM  wrote:

URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_task.h?rev=1888446=auto


[snip]

A couple of really minor nits:


+ * During execution, a task may add further sub-tasks - equivalent to
+ * sub-function calls.  They will be executed after their parent task
+ * has been processed forming an growing tree of *isolated* tasks.


s/an/a; ("a growing tree")

[snip]


+ * Errors are detected in strictly the same order, with only the first one
+ * being returned from the task runner.  In particular, it may not be the
+ * first error that occurs timewise but only the first one encountered when
+ * walking the tree and checking the processing results for errors.  Because
+ * it takes some time make the workers cancel all outstanding tasks,


Did you mean: "...some time _to_ make the workers cancel..."


Thank you for the review. Things should be fixed in r1888523.

-- Stefan^2.


Re: Interested in making 'svn st' 10x faster?

2021-04-08 Thread Stefan Fuhrmann



On 06.04.21 21:14, Daniel Shahaf wrote:

Stefan Fuhrmann wrote on Mon, Apr 05, 2021 at 21:17:23 +0200:

+static svn_error_t *output_processed(
+  svn_task__t **task,
+  svn_cancel_func_t cancel_func,
+  void *cancel_baton,
+  apr_pool_t *result_pool,
+  apr_pool_t *scratch_pool)
+{
+  svn_task__t *current = *task;
+  root_t *root = current->root;
+  apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+  output_t *output;
+  callbacks_t *callbacks;
+
+  while (current && is_processed(current))
+{
+  svn_pool_clear(iterpool);
+  if (current->first_sub)
+{
+  current = current->first_sub;
+  output = current->output;
+  callbacks = current->parent->callbacks;
+  if (output && output->prior_parent_output)
+SVN_ERR(callbacks->output_func(
+current->parent, output->prior_parent_output,
+callbacks->output_baton,
+cancel_func, cancel_baton,
+result_pool, iterpool));


FWIW: The pre-order of outputs surprised me.  When I read that in the
docstring in r1888446 I thought it was a typo for "post-order", but it's
not.


Actually, it is mainly post-order with optional pre-order outputs.
The original docstring is wrong there. r1888523 hopefully clarifies that.


+}
+  else
+{
+  output_t *output = current->output;
+  if (output)
+{
+  svn_error_t *err = output->error;
+  output->error = SVN_NO_ERROR;
+  SVN_ERR(err);
+
+  callbacks = current->callbacks;
+  if (output->output)
+SVN_ERR(callbacks->output_func(
+current, output->output,
+callbacks->output_baton,
+cancel_func, cancel_baton,
+result_pool, iterpool));
+}
+
+  /* The output function may have added additional sub-tasks. */
+  if (!current->first_sub)
+{
+  svn_task__t *to_delete = current;
+  current = to_delete->parent;
+  remove_task(to_delete);


Error leak.  Cf. tools/dev/warn-ignored-err.sh.

Yep. Fixed in r1888520.

Thank you for the review!

-- Stefan^2.


Re: Interested in making 'svn st' 10x faster?

2021-04-07 Thread Stefan Fuhrmann

On 06.04.21 21:00, Stefan Fuhrmann wrote:


On 06.04.21 08:12, Nathan Hartman wrote:

>

tasks-used-optimized-wc-status.patch applied with some offsets, so is
against a slightly out-of-date trunk, but applied without conflicts.
But my build had many test failures:

Summary of test results:
   1176 tests PASSED
   165 tests SKIPPED
   66 tests XFAILED (16 WORK-IN-PROGRESS)
   1223 tests FAILED
Python version: 2.7.16.
SUMMARY: Some tests failed


Sorry about that.  Might be side-effects of prototypical hacks to wc_*,
which I did months ago and had almost forgotten about until last night.


Turned out to be 2 segfaults which were easy to fix.

stat_tests.py 13: timestamp behaviour is still failing for 'cleanup'.
But I think this one can wait.


I noticed, however, that while the first two patches were rooted in
subversion, the third patch was rooted in subversion/libsvn_wc, so
perhaps there are other needed changes that didn't make it into the
patch? Otherwise, something is hosed... If you'd like, I'll provide
the fails.log. Be warned, it's 2.5MB!


No worries.  It's been a bit late last night.
I will try to create a streamlined patch tomorrow.


I attached the new patch. It is much cleaner, delivers good cold cache
performance (9x for large working copies) but with hot file caches, it
is still 3.5x slower than the "final product" (for very, very large w/c).

-- Stefan^2.
Index: build.conf
===
--- build.conf	(revision 1888439)
+++ build.conf	(working copy)
@@ -395,7 +395,7 @@
 private\svn_temp_serializer.h private\svn_io_private.h
 private\svn_sorts_private.h private\svn_auth_private.h
 private\svn_string_private.h private\svn_magic.h
-private\svn_subr_private.h private\svn_mutex.h
+private\svn_subr_private.h private\svn_mutex.h  private\svn_task.h
 private\svn_thread_cond.h private\svn_waitable_counter.h
 private\svn_packed_data.h private\svn_object_pool.h private\svn_cert.h
 private\svn_config_private.h private\svn_dirent_uri_private.h
Index: subversion/include/private/svn_task.h
===
--- subversion/include/private/svn_task.h	(revision 1888446)
+++ subversion/include/private/svn_task.h	(working copy)
@@ -181,8 +181,8 @@
  * into either @a svn_task__add() or @a svn_task__add_similar() - even if
  * you use a @c NULL process baton.
  *
- * @todo Consider replace these pools with a single pool at the parent
- * and turn this into a simple getter.  We will end up with a lot fewer
+ * @todo Consider replacing these pools with a single pool at the parent
+ * and turning this into a simple getter.  We will end up with a lot fewer
  * pools that OTOH may live a lot longer.
  */
 apr_pool_t *svn_task__create_process_pool(
@@ -189,7 +189,7 @@
   svn_task__t *parent);
 
 /**
- * Append a new sub-task to the current @a task with the given
+ * Append a new sub-task to the @a current task with the given
  * @a process_pool.  The latter must have been allocated with
  * @a svn_task__create_process_pool() for @a task.
  *
@@ -205,7 +205,7 @@
  * and / or processing will take place.
  */
 svn_error_t *svn_task__add(
-  svn_task__t *task,
+  svn_task__t *current,
   apr_pool_t *process_pool,
   void *partial_output,
   svn_task__process_func_t process_func,
@@ -220,7 +220,7 @@
  * as for the current task.  This is useful for recursive tasks.
  */
 svn_error_t *svn_task__add_similar(
-  svn_task__t *task,
+  svn_task__t *current,
   apr_pool_t *process_pool,
   void *partial_output,
   void *process_baton);
Index: subversion/libsvn_wc/status.c
===
--- subversion/libsvn_wc/status.c	(revision 1888427)
+++ subversion/libsvn_wc/status.c	(working copy)
@@ -51,6 +51,7 @@
 #include "private/svn_wc_private.h"
 #include "private/svn_fspath.h"
 #include "private/svn_editor.h"
+#include "private/svn_task.h"
 
 
 /* The file internal variant of svn_wc_status3_t, with slightly more
@@ -75,6 +76,28 @@
 } svn_wc__internal_status_t;
 
 
+/* Forward declare */
+struct walk_status_baton;
+
+typedef svn_error_t *
+(*dir_status_func_t)(const struct walk_status_baton *wb,
+ const char *local_abspath,
+ svn_boolean_t skip_this_dir,
+ const char *parent_repos_root_url,
+ const char *parent_repos_relpath,
+ const char *parent_repos_uuid,
+ const struct svn_wc__db_info_t *dir_info,
+ const svn_io_dirent2_t *dirent,
+ const apr_array_header_t *ignore_patterns,
+ svn_depth_t depth,
+ svn_boolean_t get_all,
+ svn_boolean_t no_ignore,
+ svn_wc_status_func4_t status_func,
+ void *status_baton

Re: Interested in making 'svn st' 10x faster?

2021-04-06 Thread Stefan Fuhrmann



On 06.04.21 08:12, Nathan Hartman wrote:

On Mon, Apr 5, 2021 at 3:17 PM Stefan Fuhrmann  wrote:


See attachment (I may have used a somewhat outdated trunk).

tasks-prepwork.patch refactors some utility code to make it reusable.
tasks-main.patch is the new svn_task__t API, implementation and bits of testing.
tasks-used*.patch is calling the new API from svn_wc__internal_walk_status plus
optimizations in wc_*.*

The patches are only to give you an idea of the scope of the change; they lack
almost all documentation.  I plan to go for a simpler first integration, i.e.
keep wc_*.* as is and possibly reduce the code churn in status.c.



Which version of APR are you using in your builds?


Nothing special - whatever comes with the distro I'm using.
Specifically 1.6.5 but I have been building & testing against 2.0.0 as well.


I used apr-1.4.8, apr-util-1.4.1 because recently there was some work
to restore building SVN with APR 1.4 to allow building on older OS
like CentOS 7. I wanted to make sure these changes don't "un-restore"
building with APR 1.4. In my build, APR_HAS_THREADS was defined.


I don't expect any specific dependency for my additions. The big-O thing
I found is a UNIX-only API detail (full path walk for every stat call
when listing a directory) and the runtime impact is moderate.  Haven't come
around to polishing up the patch, yet.


It looks like there's no fallback in case the new implementation can't
be used; e.g., svn_thread_cond__wait() et al basically do nothing
unless APR_HAS_THREADS is defined.


That is basically in keeping with how we treat svn_mutex__t.  If there
are (supposed to be) no threads, there is no point in synchronizing
and things become no-ops.  But you don't need to litter the SVN code
base with dependencies on APR_HAS_THREADS.

Today's commits should make this a bit clearer.


tasks-used-optimized-wc-status.patch applied with some offsets, so is
against a slightly out-of-date trunk, but applied without conflicts.
But my build had many test failures:

Summary of test results:
   1176 tests PASSED
   165 tests SKIPPED
   66 tests XFAILED (16 WORK-IN-PROGRESS)
   1223 tests FAILED
Python version: 2.7.16.
SUMMARY: Some tests failed


Sorry about that.  Might be side-effects of prototypical hacks to wc_*,
which I did months ago and had almost forgotten about until last night.


I noticed, however, that while the first two patches were rooted in
subversion, the third patch was rooted in subversion/libsvn_wc, so
perhaps there are other needed changes that didn't make it into the
patch? Otherwise, something is hosed... If you'd like, I'll provide
the fails.log. Be warned, it's 2.5MB!


No worries.  It's been a bit late last night.
I will try to create a streamlined patch tomorrow.

For now, you can have a look at r1888446 and see if the general API
description makes sense to you.

-- Stefan^2.


Interested in making 'svn st' 10x faster?

2021-04-05 Thread Stefan Fuhrmann

On 05.04.21 20:19, Nathan Hartman wrote:
On Mon, Apr 5, 2021 at 6:00 AM Stefan Fuhrmann <mailto:stef...@apache.org>> wrote:


Hi all,

Way back in 2014, I started work on an SVN equivalent to apr_thread_pool
and came back to it recently.  The key features are output and callbacks
happen in the same order as in sequential code, same for any svn_error_t
raised, and a low-overhead single-threaded code path.

Now, there is a working prototype and I used it in
svn_wc__internal_walk_status.
   Integration was simple enough and scaling is excellent, both for cold 
and hot
file caches.  I'm on staycation this week, so I would like to bring the code
to trunk (with proper documentation and all).  But beyond that, I don't know
how responsive I can be in case things go wrong.  So, I would very much like
somebody dedicate time to reviewing it this week, give feedback etc.

Any takers?

-- Stefan^2.



Now is actually a good time to introduce this because we have other heavy duty 
optimizations on trunk, and it would be a good thing if we could get back on 
track with regular feature releases between LTS releases, with something 
"marketable" in them, which I think this qualifies as. 1.15 could be summed up 
as a "faster performance" release.


That is good to hear.  While looking for 'svn st' performance, I also identified
a couple of redundant queries in wc_* as well as a big-O blunder in APR (will
send them a patch tomorrow).  Downside is that the attached patches are not
particularly clean.

The only reason I can think of why anyone might object is if the changes add new 
dependencies.


None of that.  The trickiest thing is how to feed the number of workers to use
into the function.  I have no real opinion on that and hard-coded it in the
svn_task__run() call instead of bumping the status API and adding a new CLI
option.

If you could post a patch of what you have so far, even if incomplete, it will 
help us see how big (or small) the proposed changes are and what they entail.


See attachment (I may have used a somewhat outdated trunk).

tasks-prepwork.patch refactors some utility code to make it reusable.
tasks-main.patch is the new svn_task__t API, implementation and bits of testing.
tasks-used*.patch is calling the new API from svn_wc__internal_walk_status plus
optimizations in wc_*.*

The patches are only to give you an idea of the scope of the change; they lack
almost all documentation.  I plan to go for a simpler first integration, i.e.
keep wc_*.* as is and possibly reduce the code churn in status.c.

I'm willing to give the code an extra pair of eyeballs, build and test, and if 
it gets the blessing of the current test suite I'm willing to build trunk with 
the new code and try it as my daily driver to put it through its paces in real 
world (client side) usage.


Thank you very much!  I hope the patches don't miss anything and you can compile
and run them as-is.

If you're concerned that you won't be able to work on it after this week, you 
could opt to put it on a branch, but I understand that it might languish there. 
Your call...


I'll go for trunk.  Committing the content of the 2 first patches as a series
of nice commits should be close to zero risk.  Depending on the feedback, we
can then decide how / when / if to continue with touching svn_wc.

-- Stefan^2.


Index: include/private/svn_task.h
===
--- include/private/svn_task.h	(nonexistent)
+++ include/private/svn_task.h	(working copy)
@@ -0,0 +1,89 @@
+/* svn_task.h : Interface to the parallel task handling.
+ *
+ * 
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements.  See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership.  The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *KIND, either express or implied.  See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ * 
+ */
+
+#ifndef SVN_TASK_H
+#define SVN_TASK_H
+
+#include "svn_pools.h"
+#include "svn_error.h"
+
+typedef struct svn_task__t svn_task__t;
+
+typedef svn_error_t *(*svn_task__process_func_t)(
+  void **result,
+  svn_task__t *task,
+  void *thread_conte

Interested in making 'svn st' 10x faster?

2021-04-05 Thread Stefan Fuhrmann

Hi all,

Way back in 2014, I started work on an SVN equivalent to apr_thread_pool
and came back to it recently.  The key features are output and callbacks
happen in the same order as in sequential code, same for any svn_error_t
raised, and a low-overhead single-threaded code path.

Now, there is a working prototype and I used it in svn_wc__internal_walk_status. 
 Integration was simple enough and scaling is excellent, both for cold and hot

file caches.  I'm on staycation this week, so I would like to bring the code
to trunk (with proper documentation and all).  But beyond that, I don't know
how responsive I can be in case things go wrong.  So, I would very much like
somebody dedicate time to reviewing it this week, give feedback etc.

Any takers?

-- Stefan^2.


Re: svn commit: r1886460 - /subversion/trunk/subversion/tests/cmdline/mod_authz_svn_tests.py

2021-02-17 Thread Stefan Sperling
On Tue, Feb 16, 2021 at 08:18:04PM +, Daniel Shahaf wrote:
> Stefan Sperling wrote on Tue, Feb 16, 2021 at 12:12:35 +0100:
> > On Mon, Feb 15, 2021 at 07:47:48PM +, Daniel Shahaf wrote:
> > > s...@apache.org wrote on Fri, Feb 12, 2021 at 10:40:16 -:
> > > > Author: stsp
> > > > Date: Fri Feb 12 10:40:16 2021
> > > > New Revision: 1886460
> > > > 
> > > > URL: http://svn.apache.org/viewvc?rev=1886460=rev
> > > > Log:
> > > > Add a test for the NULL deref issue also known as CVE-2020-17525.
> > > > 
> > > > * subversion/tests/cmdline/mod_authz_svn_tests.py
> > > >   (nonexistent_repos_relative_access_file): New test.
> > > 
> > > Propose this for backport?
> > 
> > Yes, done now in r1886583.
> 
> Thanks.  How about 1.10 too?  It received the fix so it should receive its 
> tests.

Indeed, I forgot about 1.10. Thanks for the reminder. Done now.


Re: svn commit: r1886396 - in /subversion/site/publish: ./ doap.rdf docs/release-notes/release-history.html download.html

2021-02-16 Thread Stefan Sperling
On Tue, Feb 16, 2021 at 01:05:32PM +0100, Daniel Sahlberg wrote:
> Den tis 16 feb. 2021 kl 11:34 skrev Stefan Sperling :
> > On Mon, Feb 15, 2021 at 07:46:08PM +, Daniel Shahaf wrote:
> > > The entity referred to by the  tag wasn't created in 2021.  So,
> > > I think the hunk is incorrect… but so was the original value, which 
> > > referred to
> > > the _file_'s creation date (r1053461), rather than to the date Subversion 
> > > was
> > > founded (2000), the date it was accepted into the Incubator, or the date 
> > > it was
> > > promoted to TLP.
> 
> Should this be reverted, maybe even back to the proper creation date
> (2000-02-29)?

Yes please. I'm sorry for my mistake.
Could you handle that as well while committing the change below?

> What do you think about this?
> [[[
> List the new release on ^/subversion/site/publish/doap.rdf
> There should be a  section for each supported minor release
> with the  and  being updated to the current release
> date and patch release number.
> Do not change anything else in the file (in particular the 
> under  is the date when the Subversion project was created).
> ]]]

That is crystal clear and should avoid mistakes going forward. Thank you!


Re: svn commit: r1886460 - /subversion/trunk/subversion/tests/cmdline/mod_authz_svn_tests.py

2021-02-16 Thread Stefan Sperling
On Mon, Feb 15, 2021 at 07:47:48PM +, Daniel Shahaf wrote:
> s...@apache.org wrote on Fri, Feb 12, 2021 at 10:40:16 -:
> > Author: stsp
> > Date: Fri Feb 12 10:40:16 2021
> > New Revision: 1886460
> > 
> > URL: http://svn.apache.org/viewvc?rev=1886460=rev
> > Log:
> > Add a test for the NULL deref issue also known as CVE-2020-17525.
> > 
> > * subversion/tests/cmdline/mod_authz_svn_tests.py
> >   (nonexistent_repos_relative_access_file): New test.
> 
> Propose this for backport?

Yes, done now in r1886583.

> > +++ subversion/trunk/subversion/tests/cmdline/mod_authz_svn_tests.py Fri 
> > Feb 12 10:40:16 2021
> > @@ -1072,6 +1072,43 @@ def repos_relative_access_file(sbox):
> >  
> >verify_gets(test_area_url, in_repos_authz_tests)
> >  
> > +# test for the bug also known as CVS-2020-17525
> 
> s/S/E/

Thank you! This typo is also fixed now and part of the backport proposal.

Thanks,
Stefan


Re: svn commit: r1886396 - in /subversion/site/publish: ./ doap.rdf docs/release-notes/release-history.html download.html

2021-02-16 Thread Stefan Sperling
On Mon, Feb 15, 2021 at 07:46:08PM +, Daniel Shahaf wrote:
> s...@apache.org wrote on Wed, Feb 10, 2021 at 20:39:23 -:
> > Author: stsp
> > Date: Wed Feb 10 20:39:22 2021
> > New Revision: 1886396
> > 
> > URL: http://svn.apache.org/viewvc?rev=1886396=rev
> > Log:
> > site/publish: Merge from staging area.
> 
> For future reference, this commit should have used the "less than 24 hours 
> ago" syntax:
> 
> % cd site/publish
> % grep -R -h -9  | vipe
>   
> % 
> 
> More below.
> 
> > +++ subversion/site/publish/doap.rdf Wed Feb 10 20:39:22 2021
> > @@ -22,7 +22,7 @@
> >  limitations under the License.
> >  -->
> >http://subversion.apache.org/;>
> > -2010-12-28
> > +2021-02-10
> 
> Huh?
> 
> Quoting http://usefulinc.com/ns/doap:
> 
> > > http://usefulinc.com/ns/doap#created;>
> > >   http://usefulinc.com/ns/doap#; />
> > >   created
> > >   Date when something was created, in 
> > > -MM-DD form. e.g. 2004-04-05
> > > ⋮
> > > 
> 
> The entity referred to by the  tag wasn't created in 2021.  So,
> I think the hunk is incorrect… but so was the original value, which referred 
> to
> the _file_'s creation date (r1053461), rather than to the date Subversion was
> founded (2000), the date it was accepted into the Incubator, or the date it 
> was
> promoted to TLP.
> 
> Thanks for RMing,
> 
> Daniel
> 

Thanks for checking.

I think in both of these cases it would have helped to have more specific
instructions for how to update these files in our release manager's manual
of the community guide.

Thanks,
Stefan



Re: strange error, file causes commit error

2021-02-15 Thread Stefan Sperling
On Sun, Feb 14, 2021 at 05:28:06PM -0600, Greg Stein wrote:
> Hey all,
> 
> This is a very strange error that we're seeing in Infra, on the ASF svn
> server. It appears that the presence of a particular file in a commit
> causes a failure.
> 
> Please see:
> https://issues.apache.org/jira/browse/INFRA-21346
> 
> Does anybody have thoughts on this? I'm stumped.

Not sure if it applies here, but if a write-through proxy is involved then
this issue could be relevant:
https://issues.apache.org/jira/browse/SVN-3445
"WebDAV proxy code munging user data"


Re: svn commit: r1886490 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/cmdline/ tests/libsvn_subr/ tests/libsvn_wc/

2021-02-13 Thread Stefan Sperling
On Sat, Feb 13, 2021 at 04:50:10PM +0100, Branko Čibej wrote:
> On 13.02.2021 15:32, Evgeny Kotkov wrote:
> > My attempt to fix this is by making checkout stream data to both pristine 
> > and
> > the (projected) working file, so that the actual install would then happen 
> > as
> > just an atomic rename.  Since we now never stop reading from the connection,
> > the timeouts should no longer be an issue.  The new approach also has 
> > several
> > nice properties, such as not having to re-read the pristine files, not
> > interfering with the network-level buffering, TCP slow starts, and etc.
> > I see that it reduces the amount of both read and write I/O during all
> > checkouts, which should give a mild overall increase of how fast the
> > checkouts work.
> 
> I like this concept very much indeed.

Agreed, yes! I believe I've seen reports of connections timing out
during checkout/update, which weren't understood and couldn't be
solved in other ways than bumping up server-side limits.
This might explain what was going on there. Thanks Evgeny!


Re: Returned post for annou...@apache.org

2021-02-11 Thread Stefan Sperling
On Thu, Feb 11, 2021 at 11:02:32AM +, Private List Moderation wrote:
> Irrelevant.

Given that this discussion doesn't seem to be going anywhere and the
same arguments from May 2020 are just being rehashed, I guess we will
simply stop using the announce@ mailing list.


Re: Returned post for annou...@apache.org

2021-02-11 Thread Stefan Sperling
On Thu, Feb 11, 2021 at 10:31:24AM +0100, Daniel Sahlberg wrote:
> Den tors 11 feb. 2021 kl 10:11 skrev Stefan Sperling :
> 
> > On Thu, Feb 11, 2021 at 09:57:23AM +0100, Daniel Sahlberg wrote:
> > > It is indeed a chicken-and-egg problem. However I would suggest to split
> > > this "updating the website" into two separate parts:
> > > 1. Updating the download page and possibly also adding a news item (with
> > a
> > > placeholder link to the release announcement) before the announcement.
> > > 2. After the release announcement has landed in lists.a.o, update the
> > news
> > > item with a proper link.
> > >
> > > If the website is updated in one batch after the announcement someone
> > might
> > > receive the announcement, immediately heading to the download page and
> > then
> > > not finding the announced release.
> >
> > Yes, that makes sense. Would you have time to update the web site
> > accordingly?
> >
> 
> Sure, I'll make a suggestion in staging. Not today though.
> 
> 
> > Having just run through the steps, I must say that our release manager's
> > guide
> > has a rather low signal-to-noise ratio in its current form.
> > It may help to break some text into more paragraphs with meaningful
> > headers,
> > and/or perhaps put boxes around some of the details that don't really
> > matter
> > when someone is trying to get an overview of the procedure.
> >
> 
> Since I'm new to the party, maybe I can try to understand the procedure and
> break it up, and you can tell me when I go wrong?
> 
> Kind regards,
> Daniel

Yes, sure. I will gladly review what you come up with.

Cheers,
Stefan


Re: Returned post for annou...@apache.org

2021-02-11 Thread Stefan Sperling
On Thu, Feb 11, 2021 at 09:57:23AM +0100, Daniel Sahlberg wrote:
> It is indeed a chicken-and-egg problem. However I would suggest to split
> this "updating the website" into two separate parts:
> 1. Updating the download page and possibly also adding a news item (with a
> placeholder link to the release announcement) before the announcement.
> 2. After the release announcement has landed in lists.a.o, update the news
> item with a proper link.
> 
> If the website is updated in one batch after the announcement someone might
> receive the announcement, immediately heading to the download page and then
> not finding the announced release.

Yes, that makes sense. Would you have time to update the web site accordingly?

Having just run through the steps, I must say that our release manager's guide
has a rather low signal-to-noise ratio in its current form.
It may help to break some text into more paragraphs with meaningful headers,
and/or perhaps put boxes around some of the details that don't really matter
when someone is trying to get an overview of the procedure.

Regards,
Stefan


Re: Returned post for annou...@apache.org

2021-02-10 Thread Stefan Sperling
On Wed, Feb 10, 2021 at 11:03:39PM -0500, Nathan Hartman wrote:
> On Wed, Feb 10, 2021 at 7:51 PM Private List Moderation <
> mod-priv...@gsuite.cloud.apache.org> wrote:
> > When I checked the download page, there were no links for versions 1.10.7
> > or 1.14.1.
> > i.e. the 2 announce mails were telling people to download versions that
> > were not on the download page.
> >
> > As such, I felt I had to reject the announce email.
> >
> > It looks as though the page has since been updated.
> >
> 
> 
> That was a race condition.

Worse, it's a chicken-and-egg problem which is documented here:
http://subversion.apache.org/docs/community-guide/releasing.html#releasing-release
"""
NOTE: We announce the release before updating the website since the
website update links to the release announcement sent to the announce@
mailing list.
"""

Notwithstanding that, yes, I did actually forget to update the download
page immediately after posting the announcements. dsahlberg kindly reminded
me of this. It still don't see how this is a reason for blocking a legitimate
release annoucement. Instead of rejecting the annoucement outright you could
have mailed us on dev@ with a question like "This looks incomplete. Are you
sure you want to post this annoucement?". That would have been helpful, but
moderation rejection is not helpful.

Not everyone relies on the download pages to find our releases.
At the moment I sent the announcement at least one distribution (Suse Linux)
already had RPMs with the security fix ready to go because we had sent security
pre-notifications in private about a week earlier.

Cheers,
Stefan


Re: svn commit: r1886389 - in /subversion/site/publish: ./ index.html

2021-02-10 Thread Stefan Sperling
On Wed, Feb 10, 2021 at 08:30:52PM +0100, Daniel Sahlberg wrote:
> *grabbing a commit in the pile*
> 
> Is it intentional that the download page still links to 1.14.0 (and 1.10.6)?
> 
> (I'm not familiar with the relase workflow and I realise this may be a soak
> for all mirrors to pick up the relase, just wanted to make sure it has not
> been forgotten).
> 
> Kind regards
> Daniel Sahlberg

Indeed. I had missed some of the necessary web site updates, going over the
release process docs too quickly. I have done the missing updates just now.
Hopefully everything is now in order.

Thank you!
Stefan


Re: Returned post for annou...@apache.org

2021-02-10 Thread Stefan Sperling
Sebb, blocking our release announcements over trivialities like this
really is not a nice thing to do. Last time it happened in May 2020.
It was already discussed back then and raised with the announce@
moderation team.

The Subversion PMC came to the conclusion that our handling of
the KEYS files is adequate for our purposes:
https://svn.haxx.se/dev/archive-2020-05/0156.shtml

Please raise the issue on our dev@subversion.a.o list if it bothers you.
The moderation mechanism is supposed to prevent spam. Using it to enforce
release workflow policies amounts to misuse of your moderation privileges.

Regards,
Stefan

On Wed, Feb 10, 2021 at 03:20:41PM -, announce-ow...@apache.org wrote:
> 
> Hi! This is the ezmlm program. I'm managing the
> annou...@apache.org mailing list.
> 
> I'm working for my owner, who can be reached
> at announce-ow...@apache.org.
> 
> I'm sorry, your message (enclosed) was not accepted by the moderator.
> If the moderator has made any comments, they are shown below.
> 
> >>>>>  >>>>>
> Sorry, but the announce cannot be accepted.
> The linked download page does not contain links for the version in the
> email.
> 
> Also, the standard name for the KEYS file is KEYS - no prefix, no suffix.
> Please correct the download page, check it, and submit a corrected announce
> mail.
> 
> Thanks,
> Sebb.
> <<<<<  <<<<<
> 

> Date: Wed, 10 Feb 2021 14:37:00 +0100
> From: Stefan Sperling 
> To: annou...@subversion.apache.org, us...@subversion.apache.org,
>  dev@subversion.apache.org, annou...@apache.org
> Cc: secur...@apache.org, oss-secur...@lists.openwall.com,
>  bugt...@securityfocus.com
> Subject: [SECURITY][ANNOUNCE] Apache Subversion 1.10.7 released
> Message-ID: 
> Reply-To: us...@subversion.apache.org
> Content-Type: text/plain; charset=utf-8
> 
> I'm happy to announce the release of Apache Subversion 1.10.7.
> Please choose the mirror closest to you by visiting:
> 
> https://subversion.apache.org/download.cgi#supported-releases
> 
> This is a stable bugfix and security release of the Apache Subversion
> open source version control system.
> 
> THIS RELEASE CONTAINS AN IMPORTANT SECURITY FIX:
> 
>   CVE-2020-17525
>   "Remote unauthenticated denial-of-service in Subversion mod_authz_svn"
> 
> The full security advisory for CVE-2020-17525 is available at:
>   https://subversion.apache.org/security/CVE-2020-17525-advisory.txt
> 
> A brief summary of this advisory follows:
> 
>   Subversion's mod_authz_svn module will crash if the server is using
>   in-repository authz rules with the AuthzSVNReposRelativeAccessFile
>   option and a client sends a request for a non-existing repository URL.
> 
>   This can lead to disruption for users of the service.
> 
>   We recommend all users to upgrade to the 1.10.7 or 1.14.1 release
>   of the Subversion mod_dav_svn server.
> 
>   As a workaround, the use of in-repository authz rules files with
>   the AuthzSVNReposRelativeAccessFile can be avoided by switching
>   to an alternative configuration which fetches an authz rules file
>   from the server's filesystem, rather than from an SVN repository.
> 
>   This issue was reported by Thomas Åkesson.
> 
> SHA-512 checksums are available at:
> 
> https://www.apache.org/dist/subversion/subversion-1.10.7.tar.bz2.sha512
> https://www.apache.org/dist/subversion/subversion-1.10.7.tar.gz.sha512
> https://www.apache.org/dist/subversion/subversion-1.10.7.zip.sha512
> 
> PGP Signatures are available at:
> 
> https://www.apache.org/dist/subversion/subversion-1.10.7.tar.bz2.asc
> https://www.apache.org/dist/subversion/subversion-1.10.7.tar.gz.asc
> https://www.apache.org/dist/subversion/subversion-1.10.7.zip.asc
> 
> For this release, the following people have provided PGP signatures:
> 
>Stefan Sperling [2048R/4F7DBAA99A59B973] with fingerprint:
> 8BC4 DAE0 C5A4 D65F 4044  0107 4F7D BAA9 9A59 B973
>Branko Čibej [4096R/1BCA6586A347943F] with fingerprint:
> BA3C 15B1 337C F0FB 222B  D41A 1BCA 6586 A347 943F
>Johan Corveleyn [4096R/B59CE6D6010C8AAD] with fingerprint:
> 8AA2 C10E EAAD 44F9 6972  7AEA B59C E6D6 010C 8AAD
> 
> These public keys are available at:
> 
> https://www.apache.org/dist/subversion/subversion-1.10.7.KEYS
> 
> Release notes for the 1.10.x release series may be found at:
> 
> https://subversion.apache.org/docs/release-notes/1.10.html
> 
> You can find the list of changes between 1.10.7 and earlier versions at:
> 
> https://svn.apache.org/repos/asf/subversion/tags/1.10.7/CHANGES
> 
> Questions, comments, and bug reports to us...@subversion.apache.org.
> 
> Thanks,
> - The Subversion Team
> 
> --
> To unsubscribe, please see:
> 
> https://subversion.apache.org/mailing-lists.html#unsubscribing
> 



[announce-ow...@apache.org: Returned post for annou...@apache.org]

2021-02-10 Thread Stefan Sperling
Same thing as last time. Shrug.
I suppose next time I will just drop announce@ from Cc...


- Forwarded message from announce-ow...@apache.org -

Date: 10 Feb 2021 15:20:01 -
From: announce-ow...@apache.org
To: s...@apache.org
Subject: Returned post for annou...@apache.org
Message-ID: <1612970401.10142.ez...@apache.org>
Content-Type: multipart/mixed; boundary=ibhjpejjlbghgodohoih
X-Spam-Score: (-7.502) SPF_HELO_PASS,SPF_PASS,USER_IN_DEF_SPF_WL


Hi! This is the ezmlm program. I'm managing the
annou...@apache.org mailing list.

I'm working for my owner, who can be reached
at announce-ow...@apache.org.

I'm sorry, your message (enclosed) was not accepted by the moderator.
If the moderator has made any comments, they are shown below.

>>>>>  >>>>>
Sorry, but the announce cannot be accepted.
The linked download page does not contain links for the version in the
email.

Also, the standard name for the KEYS file is KEYS - no prefix, no suffix.
Please correct the download page, check it, and submit a corrected announce
mail.

Thanks,
Sebb.
<<<<<  <<<<<


Date: Wed, 10 Feb 2021 14:36:33 +0100
From: Stefan Sperling 
To: annou...@subversion.apache.org, us...@subversion.apache.org,
 dev@subversion.apache.org, annou...@apache.org
Cc: secur...@apache.org, oss-secur...@lists.openwall.com,
 bugt...@securityfocus.com
Subject: [SECURITY][ANNOUNCE] Apache Subversion 1.14.1 released
Message-ID: 
Reply-To: us...@subversion.apache.org
Content-Type: text/plain; charset=utf-8

I'm happy to announce the release of Apache Subversion 1.14.1.
Please choose the mirror closest to you by visiting:

https://subversion.apache.org/download.cgi#recommended-release

This is a stable bugfix and security release of the Apache Subversion
open source version control system.

THIS RELEASE CONTAINS AN IMPORTANT SECURITY FIX:

  CVE-2020-17525
  "Remote unauthenticated denial-of-service in Subversion mod_authz_svn"

The full security advisory for CVE-2020-17525 is available at:
  https://subversion.apache.org/security/CVE-2020-17525-advisory.txt

A brief summary of this advisory follows:

  Subversion's mod_authz_svn module will crash if the server is using
  in-repository authz rules with the AuthzSVNReposRelativeAccessFile
  option and a client sends a request for a non-existing repository URL.

  This can lead to disruption for users of the service.

  We recommend all users to upgrade to the 1.10.7 or 1.14.1 release
  of the Subversion mod_dav_svn server.

  As a workaround, the use of in-repository authz rules files with
  the AuthzSVNReposRelativeAccessFile can be avoided by switching
  to an alternative configuration which fetches an authz rules file
  from the server's filesystem, rather than from an SVN repository.

  This issue was reported by Thomas Åkesson.

SHA-512 checksums are available at:

https://www.apache.org/dist/subversion/subversion-1.14.1.tar.bz2.sha512
https://www.apache.org/dist/subversion/subversion-1.14.1.tar.gz.sha512
https://www.apache.org/dist/subversion/subversion-1.14.1.zip.sha512

PGP Signatures are available at:

https://www.apache.org/dist/subversion/subversion-1.14.1.tar.bz2.asc
https://www.apache.org/dist/subversion/subversion-1.14.1.tar.gz.asc
https://www.apache.org/dist/subversion/subversion-1.14.1.zip.asc

For this release, the following people have provided PGP signatures:

   Stefan Sperling [2048R/4F7DBAA99A59B973] with fingerprint:
8BC4 DAE0 C5A4 D65F 4044  0107 4F7D BAA9 9A59 B973
   Branko Čibej [4096R/1BCA6586A347943F] with fingerprint:
BA3C 15B1 337C F0FB 222B  D41A 1BCA 6586 A347 943F
   Johan Corveleyn [4096R/B59CE6D6010C8AAD] with fingerprint:
8AA2 C10E EAAD 44F9 6972  7AEA B59C E6D6 010C 8AAD

These public keys are available at:

https://www.apache.org/dist/subversion/subversion-1.14.1.KEYS

Release notes for the 1.14.x release series may be found at:

https://subversion.apache.org/docs/release-notes/1.14.html

You can find the list of changes between 1.14.1 and earlier versions at:

https://svn.apache.org/repos/asf/subversion/tags/1.14.1/CHANGES

Questions, comments, and bug reports to us...@subversion.apache.org.

Thanks,
- The Subversion Team

--
To unsubscribe, please see:

https://subversion.apache.org/mailing-lists.html#unsubscribing



- End forwarded message -


[SECURITY][ANNOUNCE] Apache Subversion 1.10.7 released

2021-02-10 Thread Stefan Sperling
I'm happy to announce the release of Apache Subversion 1.10.7.
Please choose the mirror closest to you by visiting:

https://subversion.apache.org/download.cgi#supported-releases

This is a stable bugfix and security release of the Apache Subversion
open source version control system.

THIS RELEASE CONTAINS AN IMPORTANT SECURITY FIX:

  CVE-2020-17525
  "Remote unauthenticated denial-of-service in Subversion mod_authz_svn"

The full security advisory for CVE-2020-17525 is available at:
  https://subversion.apache.org/security/CVE-2020-17525-advisory.txt

A brief summary of this advisory follows:

  Subversion's mod_authz_svn module will crash if the server is using
  in-repository authz rules with the AuthzSVNReposRelativeAccessFile
  option and a client sends a request for a non-existing repository URL.

  This can lead to disruption for users of the service.

  We recommend all users to upgrade to the 1.10.7 or 1.14.1 release
  of the Subversion mod_dav_svn server.

  As a workaround, the use of in-repository authz rules files with
  the AuthzSVNReposRelativeAccessFile can be avoided by switching
  to an alternative configuration which fetches an authz rules file
  from the server's filesystem, rather than from an SVN repository.

  This issue was reported by Thomas Åkesson.

SHA-512 checksums are available at:

https://www.apache.org/dist/subversion/subversion-1.10.7.tar.bz2.sha512
https://www.apache.org/dist/subversion/subversion-1.10.7.tar.gz.sha512
https://www.apache.org/dist/subversion/subversion-1.10.7.zip.sha512

PGP Signatures are available at:

https://www.apache.org/dist/subversion/subversion-1.10.7.tar.bz2.asc
https://www.apache.org/dist/subversion/subversion-1.10.7.tar.gz.asc
https://www.apache.org/dist/subversion/subversion-1.10.7.zip.asc

For this release, the following people have provided PGP signatures:

   Stefan Sperling [2048R/4F7DBAA99A59B973] with fingerprint:
8BC4 DAE0 C5A4 D65F 4044  0107 4F7D BAA9 9A59 B973
   Branko Čibej [4096R/1BCA6586A347943F] with fingerprint:
BA3C 15B1 337C F0FB 222B  D41A 1BCA 6586 A347 943F
   Johan Corveleyn [4096R/B59CE6D6010C8AAD] with fingerprint:
8AA2 C10E EAAD 44F9 6972  7AEA B59C E6D6 010C 8AAD

These public keys are available at:

https://www.apache.org/dist/subversion/subversion-1.10.7.KEYS

Release notes for the 1.10.x release series may be found at:

https://subversion.apache.org/docs/release-notes/1.10.html

You can find the list of changes between 1.10.7 and earlier versions at:

https://svn.apache.org/repos/asf/subversion/tags/1.10.7/CHANGES

Questions, comments, and bug reports to us...@subversion.apache.org.

Thanks,
- The Subversion Team

--
To unsubscribe, please see:

https://subversion.apache.org/mailing-lists.html#unsubscribing


[SECURITY][ANNOUNCE] Apache Subversion 1.14.1 released

2021-02-10 Thread Stefan Sperling
I'm happy to announce the release of Apache Subversion 1.14.1.
Please choose the mirror closest to you by visiting:

https://subversion.apache.org/download.cgi#recommended-release

This is a stable bugfix and security release of the Apache Subversion
open source version control system.

THIS RELEASE CONTAINS AN IMPORTANT SECURITY FIX:

  CVE-2020-17525
  "Remote unauthenticated denial-of-service in Subversion mod_authz_svn"

The full security advisory for CVE-2020-17525 is available at:
  https://subversion.apache.org/security/CVE-2020-17525-advisory.txt

A brief summary of this advisory follows:

  Subversion's mod_authz_svn module will crash if the server is using
  in-repository authz rules with the AuthzSVNReposRelativeAccessFile
  option and a client sends a request for a non-existing repository URL.

  This can lead to disruption for users of the service.

  We recommend all users to upgrade to the 1.10.7 or 1.14.1 release
  of the Subversion mod_dav_svn server.

  As a workaround, the use of in-repository authz rules files with
  the AuthzSVNReposRelativeAccessFile can be avoided by switching
  to an alternative configuration which fetches an authz rules file
  from the server's filesystem, rather than from an SVN repository.

  This issue was reported by Thomas Åkesson.

SHA-512 checksums are available at:

https://www.apache.org/dist/subversion/subversion-1.14.1.tar.bz2.sha512
https://www.apache.org/dist/subversion/subversion-1.14.1.tar.gz.sha512
https://www.apache.org/dist/subversion/subversion-1.14.1.zip.sha512

PGP Signatures are available at:

https://www.apache.org/dist/subversion/subversion-1.14.1.tar.bz2.asc
https://www.apache.org/dist/subversion/subversion-1.14.1.tar.gz.asc
https://www.apache.org/dist/subversion/subversion-1.14.1.zip.asc

For this release, the following people have provided PGP signatures:

   Stefan Sperling [2048R/4F7DBAA99A59B973] with fingerprint:
8BC4 DAE0 C5A4 D65F 4044  0107 4F7D BAA9 9A59 B973
   Branko Čibej [4096R/1BCA6586A347943F] with fingerprint:
BA3C 15B1 337C F0FB 222B  D41A 1BCA 6586 A347 943F
   Johan Corveleyn [4096R/B59CE6D6010C8AAD] with fingerprint:
8AA2 C10E EAAD 44F9 6972  7AEA B59C E6D6 010C 8AAD

These public keys are available at:

https://www.apache.org/dist/subversion/subversion-1.14.1.KEYS

Release notes for the 1.14.x release series may be found at:

https://subversion.apache.org/docs/release-notes/1.14.html

You can find the list of changes between 1.14.1 and earlier versions at:

https://svn.apache.org/repos/asf/subversion/tags/1.14.1/CHANGES

Questions, comments, and bug reports to us...@subversion.apache.org.

Thanks,
- The Subversion Team

--
To unsubscribe, please see:

https://subversion.apache.org/mailing-lists.html#unsubscribing


Re: Subversion 1.10.7 up for testing/signing

2021-02-10 Thread Stefan Sperling
On Thu, Feb 04, 2021 at 01:57:16PM +0100, Stefan Sperling wrote:
> The 1.10.7 release artifacts are now available for testing/signing.
> Please get the tarballs from
>   https://dist.apache.org/repos/dist/dev/subversion
> and add your signatures there.

Summary: +1 to release

Tested: [bdb | fsfs] x [ra_local | ra_svn | ra_serf]
swig bindings
javahl bindings

Test results: All passed.

Platform: OpenBSD 6.8 amd64

Dependencies:
bdb:4.7.25
GNU-iconv:  1.15
apr:1.7.0
apr-util:   1.6.1
httpd:  2.4.37
serf:   1.3.9
cyrus-sasl: 2.1.25
sqlite: 3160200
lz4:1.7.5
libssl: LibreSSL 3.2.2
swig:   3.0.12
python: 3.7.5
perl:   5.30.3
ruby:   2.4.4
java:   11.0.8

Signatures:

subversion-1.10.7.tar.gz
-BEGIN PGP SIGNATURE-

iQEcBAABAgAGBQJgG+xhAAoJEE99uqmaWblzjX4H/1Iox2ltx4RjqmIMlXhdteLk
/IXcAlM3nU3bab+JiwYP/Ego3tGpmWqYP0Rp7vVvpwOYYM8U1uWtSvPED9+txZA2
XdkTAiE7QdO85pLfgTxy+W3zHoGytKqK0n8doVq2w3MoloV+KhlN320j7VOU4q74
h3EAMLPHiBxp9kKc2Xe/KqCYJiWuM4p4JiCiJI5jjUVc4/XrPQ4BhKK2XAAAMh7d
wZ+KheaamMrSBxuIubaGW/QJuOlHui5zIVJYsDoPIEKpUZGAytuWWAp56DmLM9CN
1gL0r+lwmSeIkSqMQaNLtxHVIo8uU2la6N0ZZqwElH/ugeSo19LKKi2ovzoRyaI=
=JqBj
-END PGP SIGNATURE-

subversion-1.10.7.tar.bz2
-BEGIN PGP SIGNATURE-

iQEcBAABAgAGBQJgG+xlAAoJEE99uqmaWblzpkEIAKanhKsDRXbVEbwL9imQRJNj
uL8q9hBrxDkWkXUpEJ2MaqcLj31mcz+REKmPrmYogpdLR829wRZ2VHcWEntacK06
4YWCXDyCCI5J2pCo1/iB6SdkS+F6NrPikZWN2QqOr+oRBccLBZwrNd8bfS0HMbT5
hQmh5U9UXYtCZNXL7qmgbLQ4PZMPDOZ0PMVnsGEhFozHwF/qMX14YOJpujruaRnB
xyoOAqShBSI1tzJXD200iQe19f9op/RZlYMuyEPG9zb9mdTzXsr9c28Dgv2BlJkQ
8ta0gzVON67tJGhTMi47VIizbPPyssEHOIT4CV+H9P9Ul4pPogkqwnMCrwyUcpE=
=CJxJ
-END PGP SIGNATURE-


Re: Subversion 1.14.1 up for testing/signing

2021-02-10 Thread Stefan Sperling
On Thu, Feb 04, 2021 at 01:56:21PM +0100, Stefan Sperling wrote:
> The 1.14.1 release artifacts are now available for testing/signing.
> Please get the tarballs from
>   https://dist.apache.org/repos/dist/dev/subversion
> and add your signatures there.
> 
> Thanks!

Summary: +1 to release

Tested: [bdb | fsfs] x [ra_local | ra_svn | ra_serf]
swig bindings
javahl bindings

Test results: All passed.

Platform: OpenBSD 6.8 amd64

Dependencies:
bdb:4.7.25
GNU-iconv:  1.15
apr:1.7.0
apr-util:   1.6.1
httpd:  2.4.37
serf:   1.3.9
cyrus-sasl: 2.1.25
sqlite: 3160200
lz4:1.7.5
libssl: LibreSSL 3.2.2
swig:   3.0.12
python: 3.7.5
perl:   5.30.3
ruby:   2.4.4
java:   11.0.8

Signatures:

subversion-1.14.1.tar.gz
-BEGIN PGP SIGNATURE-

iQEcBAABAgAGBQJgG+hsAAoJEE99uqmaWblzSbsH/3ZnrbDqsgCZeCtYSCxqBcSA
ESvlG450cbD0dTC2CimUJOwbGADm0kb1kh7LuXjXxf5XSrHLBAdh0D8FUCKpUdT1
6B06eyLehBWMQFaFnxiX1wkIj0LOZvbGkysw7zQOFu30JaqOBdMvckUbpJnb/Z4T
Oalf8ueClGONA2UB+BOiAUPYqvwcUKdPZvsSumOLV0O7SnwuqsPRrDdb9al/WWTt
bbU46t79ni0hvSToiDXgmS29BxF7JmQeG5oajS59QX+ygo6ikexqN0Ai9UxVAc5b
AEhVW4255TdRcOGhKHIgnhb3lHiY92y3feNqy3UsJQs+cB3j5vyDHlatkh3er8U=
=rIAK
-END PGP SIGNATURE-

subversion-1.14.1.tar.bz2
-BEGIN PGP SIGNATURE-

iQEcBAABAgAGBQJgG+hvAAoJEE99uqmaWblz4yUIAMCqnFGchPMrWNhrRLOb3oi+
vzk64LJ1h2X3MzstqIzRGb3ja3VmPcx2kQ4MmmHLc7XzXL7rMVRgPJB9NzKhs+dp
nvnTq0wS4KLjOzIiG3ump41T1qofi5ui5fbgeVEyU4py/fBDVeR0XokZ0k8HAZCj
oM166uprcSr0RfeQqnlSNEFUgIMq1hxTOa879N4aoMsFoLaMx18gjFL1RUyaz/0R
eH+EMkBk2wgGkzCWdOZlJeyp0YI6Lx4k/bHO8WXLci97tpw9t9UUtQAXTrKudShP
fP9BQqUv0uHdUAF3ZiYga6VKQicynuXev4du2vVqLoQ+BgMQXPwnbgA/FtSpeOA=
=z3Ba
-END PGP SIGNATURE-


  1   2   3   4   5   6   7   8   9   10   >