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

[Bert Huijben]

He ran the Subversion + the SharpSvn tests on it without unexpected failures, 
so I will start delivering binaries for this new platform using these patches.

Bert

Get Outlook for Android


From: Nathan Hartman 
Sent: Thursday, June 10, 2021 6:55:07 PM
To: Subversion Developers 
Subject: Re: svn commit: r1890674 - /subversion/branches/1.14.x/STATUS

On Thu, Jun 10, 2021 at 5:49 AM  wrote:
>
> Author: rhuijben
> Date: Thu Jun 10 09:49:48 2021
> New Revision: 1890674
>
> URL: http://svn.apache.org/viewvc?rev=1890674&view=rev
> Log:
> * STATUS: Nominate ARM64 support.
>
> Modified:
> subversion/branches/1.14.x/STATUS
>
> Modified: subversion/branches/1.14.x/STATUS
> URL: 
> http://svn.apache.org/viewvc/subversion/branches/1.14.x/STATUS?rev=1890674&r1=1890673&r2=1890674&view=diff
> ==
> --- subversion/branches/1.14.x/STATUS (original)
> +++ subversion/branches/1.14.x/STATUS Thu Jun 10 09:49:48 2021
> @@ -83,6 +83,13 @@ Candidate changes:
> Votes:
>   +1: danielsh
>
> + * r1890223, r1890668, r1890673
> +   Support building on Win64/ARM64.
> +   Justification:
> + At user request (via SharpSvn)
> +   votes:
> + +1: rhuijben

I'm not sure how much support this backport proposal will get since
the log for r1890673 says it is untested (for now). It would be
helpful if (at least) the user who requested it could write and
confirm that it's working for them.

Nathan

Re: svn commit: r1890223 - /subversion/trunk/subversion/svn_private_config.hw

[Bert Huijben]

Most buildsystems on Windows assumed just Win32 and the usual win64-x64, and 
ARM64 is a different beast... An on top of that you usually cross compile.

But I got confirmation that the binaries work great on these systems... It got 
through all the python+c tests except the 2 or 3 common failures we also see 
when setting up new bots.

Bert

Get Outlook for Android<https://aka.ms/AAb9ysg>


From: Daniel Shahaf 
Sent: Thursday, May 27, 2021 7:50:13 PM
To: Bert Huijben 
Cc: dev@subversion.apache.org 
Subject: Re: svn commit: r1890223 - 
/subversion/trunk/subversion/svn_private_config.hw

b...@qqmail.nl wrote on Thu, 27 May 2021 11:51 +00:00:
> We will need a few more patches to really support ARM64 builds. Once I
> get things working I’ll add a pull request.

Neither backport.pl nor backport.py support pull requests, so the RM
would have to merge it manually.

> Most likely I will release a SlikSvn client release for ARM64 with that
> support if everything comes together… But cross compiling on Windows is
> quite new for most of our dependencies, and ARM64 is new as well so
> most dependencies need patches (except for those building via CMake)

Is the relation between not needing patches and using CMake merely
correlation or also causation?

Cheers,

Daniel

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

[Bert Huijben]

Too bad both Neon and Serf support NTLM and SSPI for quite some time, so
the reason is a bit outdated... But I would still recommend enabling that
line, as there are quite some users that want to mix accounts and SSPI only
allows you access with the one configured for NTLM/SSPI.

On Fri, Jul 9, 2021 at 9:02 PM  wrote:

> Author: hartmannathan
> Date: Fri Jul  9 19:02:01 2021
> New Revision: 1891416
>
> URL: http://svn.apache.org/viewvc?rev=1891416&view=rev
> Log:
> * site/staging/faq.html: (#sspi): Fix typo: suppport -> support
>
> Modified:
> subversion/site/staging/faq.html
>
> Modified: subversion/site/staging/faq.html
> URL:
> http://svn.apache.org/viewvc/subversion/site/staging/faq.html?rev=1891416&r1=1891415&r2=1891416&view=diff
>
> ==
> --- subversion/site/staging/faq.html (original)
> +++ subversion/site/staging/faq.html Fri Jul  9 19:02:01 2021
> @@ -1717,7 +1717,7 @@ to see the section on SSPI authenticatio
>  
>
>  Without this line, browsers that support SSPI will prompt for the
> user's
> -credentials, but clients that do not suppport SSPI such as Subversion
> +credentials, but clients that do not support SSPI such as Subversion
>  will not prompt. (The current release of Neon - Subversion's HTTP
>  library - handles only basic authentication.)  Because the client never
>  asks for credentials, any action that requires authentication will fail.
>
>
>

Re: Missing pilcrows (was: Re: SVN-4874: Avoid foreign repository merge if repository UUIDs) match

[Bert Huijben]

Historic note:
A long time ago we had to be very strict not to do this, as some major
Subversion hosting provider (of which I don't know the name) just copied a
blank repository to create new repositories. So all repositories hosted
there had the same UUID.
(And they couldn't just 'fix' this, as that would immediately break other
working copies)


I really hope this comment is now long outdated and we can just enable
this


   Bert


On Mon, Apr 26, 2021 at 8:39 PM Daniel Shahaf  wrote:

> Thanks a lot for fixing these!  And kudos for giving the case-by-case
> details ☺
>

Re: Buildbot Migration

[Bert Huijben]

Hi Gavin,

I will see what I can do for the win32 Subversion bots. I recently upgraded
them to a new VM, but it took some effort to get the old buildbot scripts
up and running again.


On Tue, Nov 9, 2021 at 12:07 PM Gavin McDonald  wrote:

> Hi All.
>
> Getting back to this : https://issues.apache.org/jira/browse/INFRA-22277
>
> I'd like for the owner(s) of the mentioned nodes to upgrade and then
> contact me so we can get them hooked up to the new controller.
>
> TY
>

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

[Bert Huijben]

We have helper functions for this scenario (svn_uri__*()) that also perform the 
required escaping rules (required for url, but not for relpath)

Bert

-Original Message-
From: Stefan Sperling  
Sent: Monday, December 6, 2021 9:31 AM
To: Nathan Hartman 
Cc: Subversion Developers 
Subject: Re: Fwd: Can't create temporary file from template ... No such file or 
directory

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

RE: Windows build

[Bert Huijben]

-Original Message-
From: Nathan Hartman  
Sent: Tuesday, March 15, 2022 4:21 PM
To: Daniel Sahlberg 
Cc: Subversion Development 
Subject: Re: Windows build

On Tue, Mar 15, 2022 at 11:10 AM Daniel Sahlberg  
wrote:
>
> Hi,
>
> I'm once again restarting my efforts on a solid Windows build environment. 
> I've been checking the scripts in build/win32 and believe these might be a 
> reasonable starting point.

I still have a set of files to build Subversion and most dependencies as part 
of the SharpSvn build, which used to drive a few Windows buildbots as well. (I 
think they are still offline after the recent buildbot master upgrade).

The scripts also allow building Subversion and its dependencies on GitHub 
actions. (The SlikSvn Subversion binaries are built on that environment).

https://github.com/AmpScm/SharpSvn/ is the new canonical url, after moving away 
from open.collab.net.
(dependency scripts are all in the imports subdirectory)


I just upgraded the scripts there to build on Visual Studio 2022 (see r1899028 
for the changes on the Subversion side) as that is now the default build 
environment on GitHub's 'windows-latest' actions.


Bert

RE: Buildbot Migration

[Bert Huijben]

I upgraded the win32 buildbots to python 3.10 and new buildbot and they now try 
to connect to buildbot using the old credentials, but fail as the server 
doesn't expect the bot to connect.

Can somebody try to obtain new credentials for or re-enable the old 
credentials, so I can continue getting this SlikSvn sponsored bot back up.



Thanks,
Bert

-Original Message-
From: Nathan Hartman  
Sent: Sunday, April 10, 2022 5:33 PM
To: Subversion Developers 
Subject: Re: Buildbot Migration

On Tue, Jan 18, 2022 at 10:52 AM Nathan Hartman  
wrote:
>
> On Tue, Jan 18, 2022 at 10:45 AM Nathan Hartman 
>  wrote:
> >
> > On Sun, Jan 16, 2022 at 3:56 PM Branko Čibej  wrote:
> > >
> > > On 16.01.2022 18:33, Nathan Hartman wrote:
> > (snip)
> > > > Are "svn-w2k3-local" and "svn-x64-macosx-dgvrs" currently the 
> > > > only nonexistent nodes referenced by this script?
> > >
> > > Existent but not upgraded, at least in the case of the mac mini.
> > >
> > > Someday ...
> >
> > All the more reason to activate whatever is ready now and add others 
> > back if/when they are ready. I'll go ahead and request to do that...
>
> That's filed under INFRA-22761...


As part of the recent migration from buildbot 0.8 to 3.2 and ci.a.o to ci2.a.o, 
we have had no buildbots since late last year.

I think most of our buildbot configs should be able to run on 3.2, except for 
svn-w2k3-local and svn-x64-macosx-dgvrs, so as a stopgap measure, I tried 
commenting the aforementioned two and activating the config by renaming [3] to 
remove the "-disabled" suffix.

However, Gavin McDonald comments on INFRA-22761: "I'm afraid we still can not 
activate this config. There are no agents in which any of the builders can use 
at this point."

Any ideas what we should do to get some buildbots back online?

[1] https://issues.apache.org/jira/browse/INFRA-22277

[2] https://issues.apache.org/jira/browse/INFRA-22761

[3] 
https://svn.apache.org/repos/infra/infrastructure/buildbot2/projects/subversion.py

Cheers,
Nathan

RE: Buildbot Migration

[Bert Huijben]

Hi Gavin,

Thanks. The buildbots report that they are attached now:

[[
2022-04-22 09:43:03+0200 [-] recording hostname in twistd.hostname
2022-04-22 09:43:03+0200 [-] Starting factory 
2022-04-22 09:43:03+0200 [Broker,client] message from master: attached
2022-04-22 09:43:03+0200 [Broker,client] I have a leftover directory 
'svn-local' that is not being used by the buildmaster: you can delete it now
2022-04-22 09:43:03+0200 [Broker,client] I have a leftover directory 
'svn-test-run' that is not being used by the buildmaster: you can delete it now
2022-04-22 09:43:03+0200 [Broker,client] Connected to buildmaster; worker is 
ready
2022-04-22 09:43:03+0200 [Broker,client] sending application-level keepalives 
every 600 seconds
]]

Looks like the next step would be to re-enable the builds in the Subversion 
config. 

Nathan do you want to look at that? Or anybody else?
(I see that the Subversion config is renamed to .disabled and I assume quite a 
few builds don't apply any more)

Otherwise I will look into this, next weekend.

Thanks,
Bert!

-Original Message-
From: Gavin McDonald  
Sent: Thursday, April 21, 2022 10:18 PM
To: dev@subversion.apache.org
Subject: RE: Buildbot Migration

Hi Bert,

I emailed you some new connection details a few hours ago, let me know if your 
workers can connect and I'll see about enabling those jobs.

I have also changed the config for the ubuntu jobs to use asf hardware.

That leaves the mac builds - if nobody can source a mac I'll see what I can do 
my end

Gav...


On 2022/04/14 11:08:40 Bert Huijben wrote:
> I upgraded the win32 buildbots to python 3.10 and new buildbot and they now 
> try to connect to buildbot using the old credentials, but fail as the server 
> doesn't expect the bot to connect.
> 
> Can somebody try to obtain new credentials for or re-enable the old 
> credentials, so I can continue getting this SlikSvn sponsored bot back up.
> 
> 
> 
> Thanks,
>   Bert
> 
> -Original Message-
> From: Nathan Hartman 
> Sent: Sunday, April 10, 2022 5:33 PM
> To: Subversion Developers 
> Subject: Re: Buildbot Migration
> 
> On Tue, Jan 18, 2022 at 10:52 AM Nathan Hartman  
> wrote:
> >
> > On Tue, Jan 18, 2022 at 10:45 AM Nathan Hartman 
> >  wrote:
> > >
> > > On Sun, Jan 16, 2022 at 3:56 PM Branko Čibej  wrote:
> > > >
> > > > On 16.01.2022 18:33, Nathan Hartman wrote:
> > > (snip)
> > > > > Are "svn-w2k3-local" and "svn-x64-macosx-dgvrs" currently the 
> > > > > only nonexistent nodes referenced by this script?
> > > >
> > > > Existent but not upgraded, at least in the case of the mac mini.
> > > >
> > > > Someday ...
> > >
> > > All the more reason to activate whatever is ready now and add 
> > > others back if/when they are ready. I'll go ahead and request to do 
> > > that...
> >
> > That's filed under INFRA-22761...
> 
> 
> As part of the recent migration from buildbot 0.8 to 3.2 and ci.a.o to 
> ci2.a.o, we have had no buildbots since late last year.
> 
> I think most of our buildbot configs should be able to run on 3.2, except for 
> svn-w2k3-local and svn-x64-macosx-dgvrs, so as a stopgap measure, I tried 
> commenting the aforementioned two and activating the config by renaming [3] 
> to remove the "-disabled" suffix.
> 
> However, Gavin McDonald comments on INFRA-22761: "I'm afraid we still can not 
> activate this config. There are no agents in which any of the builders can 
> use at this point."
> 
> Any ideas what we should do to get some buildbots back online?
> 
> [1] https://issues.apache.org/jira/browse/INFRA-22277
> 
> [2] https://issues.apache.org/jira/browse/INFRA-22761
> 
> [3] 
> https://svn.apache.org/repos/infra/infrastructure/buildbot2/projects/s
> ubversion.py
> 
> Cheers,
> Nathan
> 
> 

RE: svn commit: r1902364 - /subversion/trunk/build.conf

[Bert Huijben]

> ==
> --- subversion/trunk/build.conf (original)
> +++ subversion/trunk/build.conf Thu Jun 30 08:10:48 2022
> @@ -340,6 +340,7 @@ type = ra-module
>  path = subversion/libsvn_ra_serf
>  install = serf-lib
>  libs = libsvn_delta libsvn_subr aprutil apriconv apr serf zlib
> +add-install-deps = $(SVN_FS_LIB_INSTALL_DEPS)
>  msvc-static = yes
>  
>  # Accessing repositories via SVN

Why add this dependency to libsvn_ra_serf?

I think you have a different problem in your build if you need the fs libraries 
for building the serf ra layer.

Bert

RE: svn commit: r1902364 - /subversion/trunk/build.conf

[Bert Huijben]

Most likely this user just has a bunch of libraries in that environment 
variable, and this links the ra layers to quite a few dependencies it doesn't 
need, potentially loading things like BDB in cases where it Is absolutely not 
necessary.

There are other better ways to add the depencies to specific libraries, like 
that libs line right above this. This line links the fake serf target, which 
does have the dependencies for serf... And that one would need fixing if the 
user has a problem.

-Original Message-
From: Bert Huijben  
Sent: Monday, July 4, 2022 4:07 PM
To: dev@subversion.apache.org; comm...@subversion.apache.org
Subject: RE: svn commit: r1902364 - /subversion/trunk/build.conf

> ==
> --- subversion/trunk/build.conf (original)
> +++ subversion/trunk/build.conf Thu Jun 30 08:10:48 2022
> @@ -340,6 +340,7 @@ type = ra-module
>  path = subversion/libsvn_ra_serf
>  install = serf-lib
>  libs = libsvn_delta libsvn_subr aprutil apriconv apr serf zlib
> +add-install-deps = $(SVN_FS_LIB_INSTALL_DEPS)
>  msvc-static = yes
>  
>  # Accessing repositories via SVN

Why add this dependency to libsvn_ra_serf?

I think you have a different problem in your build if you need the fs libraries 
for building the serf ra layer.

Bert

RE: svn commit: r1902364 - /subversion/trunk/build.conf

[Bert Huijben]

That line will add the ./configure time dependencies (or gen-make.py for 
Windows). The dependencies on our own libraries are added by the ‘libs =’ line 
right above.

 �

If you don’t get these, then it is most likely because the files don’t 
exist/aren’t build at the right time, and in that case you need to use a 
different make target, like you already suggested.

 �

 �

Adding more and more unneeded dependencies will make things far harder to 
diagnose in the future, so we should remove strange inner dependencies for just 
a specific environment, or use the proper systems if we ever want to be able to 
find bugs in this space.

 �

   Bert

 �

From: Daniel Sahlberg  
Sent: Monday, July 4, 2022 4:29 PM
To: Subversion Development 
Cc: comm...@subversion.apache.org
Subject: Re: svn commit: r1902364 - /subversion/trunk/build.conf

 �

Den mån 4 juli 2022 kl 16:07 skrev Bert Huijben mailto:b...@qqmail.nl> >:

> ==
> --- subversion/trunk/build.conf (original)
> +++ subversion/trunk/build.conf Thu Jun 30 08:10:48 2022
> @@ -340,6 +340,7 @@ type = ra-module
> � path = subversion/libsvn_ra_serf
> � install = serf-lib
> � libs = libsvn_delta libsvn_subr aprutil apriconv apr serf zlib
> +add-install-deps = $(SVN_FS_LIB_INSTALL_DEPS)
> � msvc-static = yes
> � 
> � # Accessing repositories via SVN

Why add this dependency to libsvn_ra_serf?

I think you have a different problem in your build if you need the fs libraries 
for building the serf ra layer.

 �

Probably :-). I think I need some pointers where to look to understand the 
build system.

 �

The problem (the longer version has been discussed here already here and is 
documented in issue #4901) is that �make install-serf-lib  fail when linking 
�libtool: warning: relinking 'libsvn_ra_serf-1.la <http://libsvn_ra_serf-1.la> 
':

/usr/bin/ld: cannot find -lsvn_delta-1
/usr/bin/ld: cannot find -lsvn_subr-1

 �

As far as I could determine these were installed by �make  install-fsmod-lib. 
But again, I'm new to this...

 �

Kind regards,

Daniel

 �

Re: svn commit: r1903814 - /subversion/branches/issue-4908/subversion/libsvn_wc/deprecated.c

[Bert Huijben]

Why the branch?
Looks like a quite obvious fix to me, so you could have just committed this
to trunk.

+1 on backporting this fix, although I would recommend users of the old
(pre 1.7) api to upgrade to the newer status apis, as that makes an insane
amount of performance difference on most platforms.

   Bert

On Thu, Sep 1, 2022 at 4:10 PM  wrote:

> Author: hartmannathan
> Date: Thu Sep  1 14:10:23 2022
> New Revision: 1903814
>
> URL: http://svn.apache.org/viewvc?rev=1903814&view=rev
> Log:
> On the 'issue-4908' branch, add the proposed fix
>
> * subversion/libsvn_wc/deprecated.c
>   (svn_wc__status2_from_3): To allow users of deprecated APIs, such as
> PySVN,
>to detect working copy locked status, copy 'locked' from old_status to
>status.
>
> Modified:
> subversion/branches/issue-4908/subversion/libsvn_wc/deprecated.c
>
> Modified: subversion/branches/issue-4908/subversion/libsvn_wc/deprecated.c
> URL:
> http://svn.apache.org/viewvc/subversion/branches/issue-4908/subversion/libsvn_wc/deprecated.c?rev=1903814&r1=1903813&r2=1903814&view=diff
>
> ==
> --- subversion/branches/issue-4908/subversion/libsvn_wc/deprecated.c
> (original)
> +++ subversion/branches/issue-4908/subversion/libsvn_wc/deprecated.c Thu
> Sep  1 14:10:23 2022
> @@ -2815,6 +2815,7 @@ svn_wc__status2_from_3(svn_wc_status2_t
>  }
>
>(*status)->entry = entry;
> +  (*status)->locked = old_status->locked;
>(*status)->copied = old_status->copied;
>(*status)->repos_lock = svn_lock_dup(old_status->repos_lock,
> result_pool);
>
>
>
>

Re: svn commit: r1903814 - /subversion/branches/issue-4908/subversion/libsvn_wc/deprecated.c

[Bert Huijben]

File externals didn't exist in the entry world, so you can ignore that edge
case. Older code will just detect them as a switched path which is fine, as
that is essentially what they are anyway.

Normal (directory) externals should be mapped, but I assume they already
are or we would have found these earlier on in the WC-NG migration.

The lock flag is seldom tested, nor seen from testcode as it would imply
the working copy is broken by some operation If we have such a
testcase, we usually fix the real issue instead of increasing test coverage.
(The issue was far more common pre WC-NG, where we didn't have true
recursive operations... so we had to lock on many levels separately)

Bert

On Fri, Sep 9, 2022 at 10:46 PM Nathan Hartman 
wrote:

> On Fri, Sep 9, 2022 at 6:31 AM Bert Huijben  wrote:
> >
> > Why the branch?
> > Looks like a quite obvious fix to me, so you could have just committed
> this to trunk.
>
> That does seem a little excessive, doesn't it? :-)
>
> I'll merge and nominate for backport when I get to a computer.
>
> I wanted to write a regression test too, but got stuck there -- was
> looking for the right place to add it and something that already
> exercises that version API to use as a starting point.
>
> The test suite (without a new regression test) didn't throw up any
> complaints.
>
> Btw, while looking at this, I found we also aren't copying the
> 'external' flag (or file_external, I don't have the name handy at the
> moment), so I'll fix that too.
>
> AFAICT we're copying/translating everything else that has a
> representation in the older struct. So, just the lock and external
> flags are missing.
>
> > +1 on backporting this fix, although I would recommend users of the old
> (pre 1.7) api to upgrade to the newer status apis, as that makes an insane
> amount of performance difference on most platforms.
>
> Cool. I'll add your vote when I nominate it...
>
> Cheers,
> Nathan
>

RE: SlikSVN/SharpSvn link on website

[Bert Huijben]

Thanks!

 �

I moved most of the projects I worked on, from open.collab.net to below 
https://github.com/ampscm/

(Help / pull requests welcome! ;-)

 �

   Bert

 �

From: Daniel Sahlberg  
Sent: Tuesday, November 29, 2022 10:30 PM
To: Mark Phippard 
Cc: Subversion Development ; rhuij...@apache.org
Subject: Re: SlikSVN/SharpSvn link on website

 �

Den tis 22 nov. 2022 kl 14:33 skrev Mark Phippard < <mailto:markp...@gmail.com> 
markp...@gmail.com>:

On Mon, Nov 21, 2022 at 5:47 AM Daniel Sahlberg
< <mailto:daniel.l.sahlb...@gmail.com> daniel.l.sahlb...@gmail.com> wrote:
>
> Hi,
>
> On the Binary Packages page [1], under Windows, there is a link to SlikSVN: 
> "...maintained by Bert Huijben, SharpSvn project".
>
> The name and project links to  <http://open.collab.net> open.collab.net, 
> which currently doesn't work (outdated SSL protocol version).
>
> Shall we replace this text with another link? I found a github page [2] which 
> seems to be the successor.

I'd suggest you just make the change and let Bert followup if he
chooses. There is no value in maintaining the current links that have
not been valid for years.

 �

Thanks. I allowed it to soak for one more week but committed �r1905623 tonight.

 �

/Daniel

 �

RE: svn commit: r1905663 - in /subversion/branches/pristines-on-demand-on-mwf/subversion: include/ include/private/ libsvn_client/ libsvn_wc/

[Bert Huijben]

All the now deprecated functions now fail unconditionally when the setting is 
enabled. Isn’t it possible to do this more graceful whenever a file is 
encountered which misses it’s prisite version?

 �

As far as I know it is expected that some of the files do have pristines, while 
others don’t… That would allow things like diffs on old clients that didn’t 
switch apis yet.

 �

 �

And in many cases these clients might just pass a wc-ctx that was created with 
the client api, so they might even have support for obtaining the pristines via 
callbacks without even knowing… but now they fail because we just check a 
single boolean in the deprecated api.

 �

 �

I’m not even sure if this is really worth revving the apis without altering the 
arguments… We introduced new error codes on existing apis before, like when we 
switched to WC-NG.

 �

Bert

 �

From: Daniel Sahlberg  
Sent: Thursday, December 1, 2022 1:14 PM
To: dev@subversion.apache.org
Cc: comm...@subversion.apache.org
Subject: Re: svn commit: r1905663 - in 
/subversion/branches/pristines-on-demand-on-mwf/subversion: include/ 
include/private/ libsvn_client/ libsvn_wc/

 �

Den tors 1 dec. 2022 kl 11:42 skrev mailto:kot...@apache.org> >:

Author: kotkov
Date: Thu Dec � 1 10:42:41 2022
New Revision: 1905663

 �

[...] �

 �

Modified: 
subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h
URL: 
http://svn.apache.org/viewvc/subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h?rev=1905663
 

 &r1=1905662&r2=1905663&view=diff
==
--- 
subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h
 (original)
+++ 
subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h
 Thu Dec � 1 10:42:41 2022
@@ -581,6 +581,12 @@ SVN_ERROR_START
 �  �  �  �  �  �  � SVN_ERR_WC_CATEGORY_START + 42,
 �  �  �  �  �  �  � "Incompatible working copy settings")

+ � /** @since New in 1.15 */
+ � SVN_ERRDEF(SVN_ERR_WC_DEPRECATED_API_STORE_PRISTINE,
+ �  �  �  �  �  �  �SVN_ERR_WC_CATEGORY_START + 43,
+ �  �  �  �  �  �  �"This client was not updated to support working copies "
+ �  �  �  �  �  �  �"without local pristines")
+
 �  �/* fs errors */

 �

Is it really "This client"? It looks more to be based on the WC setting.

 �

Kind regards,

/Daniel

 �

Re: svn commit: r1908545 - /subversion/trunk/build.conf

[Bert Huijben]

Those functions are not exported from the dll on Windows, -only specific marked 
functions are, ulike unix where we export everything- so a workaround was 
needed to access those functions from our test suite.


This patch will break testing on Windows, but with +- all the buildings down 
for different forms of maintenance nobody notices.

The old issue was test only, and I don't think just breaking all WC test on 
Windows just for build sanityvreasond is a proper fix. This will need a proper 
fix to allow release testing on Windows again.

Bert

Sent from Outlook for Android

From: Nathan Hartman 
Sent: Sunday, May 28, 2023 4:12:25 PM
To: Daniel Sahlberg 
Cc: Jun Omae ; phi...@apache.org ; 
dev@subversion.apache.org 
Subject: Re: svn commit: r1908545 - /subversion/trunk/build.conf

On Sun, May 28, 2023 at 5:11 AM Daniel Sahlberg
 wrote:
>
> Good catch Jun!
>
> Den sön 28 maj 2023 kl 07:33 skrev Jun Omae :
>>
>> Hi,
>>
>> On 2023/03/20 10:20, phi...@apache.org wrote:
>
> [...]
>>
>> > ==
>> > --- subversion/trunk/build.conf (original)
>> > +++ subversion/trunk/build.conf Mon Mar 20 01:20:29 2023
>> > @@ -1337,7 +1337,7 @@ msvc-force-static = yes
>> >  description = Test Sqlite query evaluation
>> >  type = exe
>> >  path = subversion/tests/libsvn_wc
>> > -sources = wc-queries-test.c ../../libsvn_subr/sqlite3wrapper.c
>> > +sources = wc-queries-test.c
>> >  install = test
>> >  libs = libsvn_test libsvn_wc libsvn_subr apriconv apr sqlite
>
>
> This reverts the first half of half of r1536364:
>
> [[[
> Don't compile the SQLite amalgamation twice. The WC test can use the same
> wrapper as libsvn_subr.
>
> * build.conf (wc-queries-test): Add sqlite3wrapper from libsvn_subr to 
> sources.
> * subversion/tests/libsvn_wc/wc-queries-test.c: Don't include sqlite3.c;
>Instead, use the same method as libsvn_subr/sqlite.c to import the
>wrapped functions from sqlite3wrapper.c
>   (test_sqlite_version): Call sqlite3_libversion instead of using the
>sqlite3_version array directly, since the latter is not exported from
>sqlite3wrapper.c.
> ]]]
>
> The code in wc-queries-test.c and sqlite.c seems to be exactly the same.
>
> @philip: What was the exact error message from the gcc sanitizer and can you 
> dig a little deeper to see if it could be resolved in another way?
>
> Kind regards,
> Daniel Sahlberg


svn_sqlite3__api_initialize, svn_sqlite3__api_funcs, etc., not being
exported under Windows build?

Or are exported with mangled names for some reason? (e.g., could
happen if built as C++ (e.g., MSVC /TP switch) and extern declarations
aren't wrapped with 'extern "C"'.)

I feel like building/linking twice was a kludge that could happen
because of something like that.

Nathan

RE: svn commit: r918542 - in /subversion/trunk/subversion/libsvn_wc: entries.c entries.h log.c old-and-busted.c update_editor.c wc-queries.sql wc_db.c wc_db.h

[Bert Huijben]

> -Original Message-
> From: Greg Stein [mailto:gst...@gmail.com]
> Sent: woensdag 3 maart 2010 17:22
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r918542 - in
> /subversion/trunk/subversion/libsvn_wc: entries.c entries.h log.c old-and-
> busted.c update_editor.c wc-queries.sql wc_db.c wc_db.h
> 
> Please revert.
> 
> The atts_to_entry stuff for the CMT_AUTHOR and friends needs to
> remain. That is how we load old entries files.

Yes, the ones in old-and-busted.c, but I didn't remove those; see the diff. 
(That flag which keeps track of what is modified there, is ignored for about
a year there).

I just set the update flag to 0 there, like we did for similar changes. I
just removed the write ones in svn_wc__loggy_entry_modify, as we are no
longer writing them in the log.

When we are reading the entire entry (like on upgrades) the code still works
as before.

Bert

RE: RE: svn commit: r918542 - in /subversion/trunk/subversion/libsvn_wc: entries.c entries.h log.c old-and-busted.c update_editor.c wc-queries.sql wc_db.c wc_db.h

[Bert Huijben]

Thanks,

 

(I think I add some very very old working copy later tonight to
automatically check what you were afraid of :))

 

Have a good flight!

 

Bert

 

From: Greg Stein [mailto:gst...@gmail.com] 
Sent: woensdag 3 maart 2010 18:40
To: Bert Huijben
Cc: dev@subversion.apache.org
Subject: Re: RE: svn commit: r918542 - in
/subversion/trunk/subversion/libsvn_wc: entries.c entries.h log.c
old-and-busted.c update_editor.c wc-queries.sql wc_db.c wc_db.h

 

Hrm. Okee... sorry then; I'll review in solid detail after plane flight,
when I'm back to my laptop. 

On Mar 3, 2010 9:35 AM, "Bert Huijben"  wrote:



> -Original Message-
> From: Greg Stein [mailto:gst...@gmail.com]
> Sent: woensdag 3 maart...

Yes, the ones in old-and-busted.c, but I didn't remove those; see the diff.
(That flag which keeps track of what is modified there, is ignored for about
a year there).

I just set the update flag to 0 there, like we did for similar changes. I
just removed the write ones in svn_wc__loggy_entry_modify, as we are no
longer writing them in the log.

When we are reading the entire entry (like on upgrades) the code still works
as before.

   Bert

RE: pristine store design

[Bert Huijben]

> -Original Message-
> From: Neels J Hofmeyr [mailto:ne...@elego.de]
> Sent: woensdag 3 maart 2010 20:26
> To: Greg Stein
> Cc: Stefan Sperling; dev@subversion.apache.org
> Subject: Re: pristine store design
> 
> Two separate things are getting mixed up here.
> 
> 1) The fact that storing (only) md5 in the DB introduces the need for
> locking where it would otherwise be lockless.

If we move the file in place from temp to final location, not overwriting
existing files (fixes Windows case), then retrieve file size and date, then

INSERT OR IGNORE INTO PRISTINE (..., ...) VALUES (..., ...)

You have enough synchronization for adding new files. 

A reader won't notice that anything happens and a simultaneous writer will
do exactly the same thing, as the MD5, size and timestamp of the file in the
pristine store will be identical for both clients.

(This will fail for the case where one writes a compressed version and one
doesn't. But if you make the compressed files use another extension you can
detect this case by checking the affected_rows value of the INSERT and
delete the file afterwards)


The real problem is: how do you handle removing files from the pristine
store with this scheme?

Possible (too) Easy fix for 1.7: Make sure you have a depth infinity lock on
the entire working copy/working copy database.

Bert

RE: holes in the WC-NG schema?

[Bert Huijben]

> -Original Message-
> From: Neels J Hofmeyr [mailto:ne...@elego.de]
> Sent: vrijdag 5 maart 2010 11:45
> To: dev@subversion.apache.org
> Subject: Re: holes in the WC-NG schema?
> 
> Neels J Hofmeyr wrote:
> > I remember someone talking of a hole. It went something like: If a
folder is
> > copied-here, all its children have locally added status, and I
understand
> > they refer to the op-root of the add to find out their history, i.e.
that
> > they are copied. Now what if I replace such a child node with a fresh,
new
> > node -- it will still think that it's part of the copy-here. Just vague
> > memory, haven't verified. This one should be fixed if it turns out to be
so.
> 
> Turns out we are aware of this problem. My noob shot at a solution would
be
> to have an indicator whether a WORKING node is the op-root of an add.
> Then
> we can have an op-root of an add within another op-root of an add without
> confusion. We might indicate on the inner op-root that they are not the
root
> of all locally added nodes, but just the root of an add operation. (Or
have
> scan_addition() find out by also scanning the parent of each add-op-root,
> which it does anyway when asked to find the repository path of the add,
> which it derives from the op-root's parent's BASE tree node ('s
ancestors).)

This would fix local adds, but it is not a complete solution for allowing
all revert scenarios from nested add with history trees. (especially the
cases where you replace some subtree of an add with history with a different
tree)

We need a more advanced schema to fix this category. The simple fix would
fix this behavior we supported in 1.6, but we might want the better fix.

Bert

RE: [PATCH] Improve single byte read stream performance

[Bert Huijben]

> -Original Message-
> From: Stefan Fuhrman [mailto:stefanfuhrm...@alice-dsl.de]
> Sent: zondag 7 maart 2010 20:40
> To: dev@subversion.apache.org
> Subject: [PATCH] Improve single byte read stream performance
> 
> While profiling TSVN's SVN proper access code,
> I found that about 40% of the runtime was spent in
> svn_config_get_config(). The reason was the config
> parser fetching individual bytes from the input stream.
> 
> However, translated_stream_read() obviously imposes
> a significant overhead per call for smaller chunk sizes.
> This patch quadruples the translated_stream_read()
> throughput for single bytes cutting svn_config_get_config()
> runtime by 50%.

Hi Stefan,

Thanks for your patch. It looks like a simple optimization that can help.

Where are those other slowdowns you see?

If we want to speed up the configuration file parsing we could also optimize
the parser in config_file.c to buffer at the character level. (This seems a
more logical place to me than to optimize all specific stream types for
single byte access). 

It might even be easier to rewrite some parts of this config parser using
the line based API we also use for reading diff files.

Bert

> 
> -- Stefan^2.
> 
> [[[
> Speed up input stream processing in config parser and
> others that read single bytes from a stream.
> 
> * subversion/libsvn_subr/subst.c
>   (translated_stream_read): Add an optimized code path
> for single byte read requests.
> 
> Patch by: Stefan Fuhrmann 
> ]]]

RE: Patch to use root letter on windows

[Bert Huijben]

> -Original Message-
> From: C. Michael Pilato [mailto:cmpil...@collab.net]
> Sent: woensdag 3 maart 2010 18:57
> To: Bert Huijben
> Cc: Hugo Bastos Weber; dev@subversion.apache.org
> Subject: Re: Patch to use root letter on windows
> 
> Bert, is this something you'd have time to look into?
> 
> Hugo Bastos Weber wrote:
> > I've sent a patch that corrects URL parser on windows. It has been moths
> and the patch still at
> >
> > http://subversion.tigris.org/issues/show_bug.cgi?id=3535
> >
> > When will this patch be commited?

This patch has now been committed as r920292, so it will be available in 1.7.0.
(A new regression test has been added to checkout_tests.py)

Thanks,
Bert

Buildbot status

[Bert Huijben]

Hi Erik,

Your buildbot reports that the regular 'svn update' skips the entire working
copy. Any idea what is going on?

---
starting svn operation
/usr/local/bin/svn update --revision 920279 --non-interactive
--no-auth-cache
 in dir /home/svnbuilder/slave/eh-debsarge1/build (timeout 3600 secs)
 watching logfiles {}
 argv: ['/usr/local/bin/svn', 'update', '--revision', '920279',
'--non-interactive', '--no-auth-cache']
 environment:
  PATH=/usr/bin:/bin:/usr/local/bin
 closing stdin
 using PTY: True
Skipped '.'
Summary of conflicts:
  Skipped paths: 1
/usr/local/bin/svnversion .
 in dir /home/svnbuilder/slave/eh-debsarge1/build
 watching logfiles {}
 argv: ['/usr/local/bin/svnversion', '.']
 environment:
  LC_ALL=C
  PATH=/usr/bin:/bin:/usr/local/bin
 closing stdin
 using PTY: False
SVN.parseGotRevision unable to parse output of svnversion: ''
program finished with exit code 0
---


Thanks,
Bert

RE: [PATCH] Replace entries in revision

[Bert Huijben]

> -Original Message-
> From: Greg Stein [mailto:gst...@gmail.com]
> Sent: dinsdag 9 maart 2010 0:39
> To: Greg Stein; dan...@elego.de; dev@subversion.apache.org
> Subject: Re: [PATCH] Replace entries in revision
> 
> Looks pretty good!
> 
> Several points:
> 
> * use svn_wc__internal_text_modified_p(); then you won't need wc_ctx
> (just the db)
> * read_info can return NULL for the repos_* values
> * typo in analyze_status docstring (the typename)
> * status_deleted nodes have no revision info; you may simply want to
> test for SVN_INVALID_REVNUM, since read_info() will return that when
> it is "not interesting". IOW, it does the proper work for you
> * do you need wc_root? how about just using
> svn_wc__internal_path_switched() ?

* You could optimize the checking for modifications to stop checking when
you find the first change. (You are not reporting specific changes)

Bert

RE: svn commit: r920526 - /subversion/trunk/subversion/libsvn_wc/update_editor.c

[Bert Huijben]

> -Original Message-
> From: Julian Foad [mailto:julianf...@btopenworld.com]
> Sent: dinsdag 9 maart 2010 11:15
> To: rhuij...@apache.org; dev@subversion.apache.org;
> comm...@subversion.apache.org
> Subject: Re: svn commit: r920526 -
> /subversion/trunk/subversion/libsvn_wc/update_editor.c
> 
> On Mon, 2010-03-08 at 21:34 +, rhuij...@apache.org wrote:
> > Author: rhuijben
> > Date: Mon Mar  8 21:34:38 2010
> > New Revision: 920526
> >
> > URL: http://svn.apache.org/viewvc?rev=920526&view=rev
> > Log:
> > * subversion/libsvn_wc/update_editor.c
> >   (install_added_props): Remove call to
> svn_wc__db_base_set_dav_cache()
> > as the only caller, svn_wc_add_repos_file4() should never change
> > BASE_NODE information. (The information that was stored before this
> > patch is from the copy from origin, which is unrelated to the
> > BASE_NODE)
> 
> Please could you update the function's doc string. It says "BASE_PROPS
> can contain entryprops and wcprops as well", implying that it does
> something with both kinds, and now it doesn't.

BASE_PROPS refers to the 'unmodified properties', to be stored in WORKING_NODE 
(See: @BASE), not to the properties of the BASE_NODE/checked out state 
(@ORIGINAL). 
This comment was written before the definition changed with WC-NG.

Bert

RE: svn commit: r920839 - /subversion/trunk/subversion/libsvn_wc/update_editor.c

[Bert Huijben]

> -Original Message-
> From: phi...@apache.org [mailto:phi...@apache.org]
> Sent: dinsdag 9 maart 2010 13:30
> To: comm...@subversion.apache.org
> Subject: svn commit: r920839 -
> /subversion/trunk/subversion/libsvn_wc/update_editor.c
> 
> Author: philip
> Date: Tue Mar  9 12:29:58 2010
> New Revision: 920839
> 
> URL: http://svn.apache.org/viewvc?rev=920839&view=rev
> Log:
> * subversion/libsvn_wc/update_editor.c
>   (cleanup_dir_baton): Use the proper wc context.
> 
> Modified:
> subversion/trunk/subversion/libsvn_wc/update_editor.c
> 
> Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/upd
> ate_editor.c?rev=920839&r1=920838&r2=920839&view=diff
> ==
> 
> --- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/update_editor.c Tue Mar  9
> 12:29:58 2010
> @@ -539,11 +539,7 @@ cleanup_dir_baton(void *dir_baton)
>   associated with a pool distinct from the edit pool and so were
>   removed separately. */
>if (!err && !eb->close_edit_complete)
> -{
> -  svn_wc_context_t fake_ctx;
> -  fake_ctx.db = eb->db;
> -  err = svn_wc__release_write_lock(&fake_ctx, db->local_abspath, pool);
> -}
> +err = svn_wc__release_write_lock(eb->wc_ctx, db->local_abspath,
> pool);
> 
>if (err)
>  {
> 
Please use svn_wc__context_create_with_db() if you need to wrap a db. (But we 
prefer not to go back to wc_ctx wherever possible, by using internal functions 
that handle only with DBs)

Bert

RE: svn commit: r921445 - in /subversion/trunk/subversion: include/private/svn_wc_private.h include/svn_wc.h libsvn_client/commit.c libsvn_wc/entries.c

[Bert Huijben]

> -Original Message-
> From: phi...@apache.org [mailto:phi...@apache.org]
> Sent: woensdag 10 maart 2010 17:47
> To: comm...@subversion.apache.org
> Subject: svn commit: r921445 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h include/svn_wc.h libsvn_client/commit.c
> libsvn_wc/entries.c
> 
> Author: philip
> Date: Wed Mar 10 16:47:27 2010
> New Revision: 921445
> 
> URL: http://svn.apache.org/viewvc?rev=921445&view=rev
> Log:
> Remove some access batons from post-commit processing.
> 
> * subversion/include/svn_wc.h
>   (svn_wc_mark_missing_deleted): Deprecate.
> 
> * subversion/include/private/svn_wc_private.h
>   (svn_wc__temp_mark_missing_not_present): New.
> 
> * subversion/libsvn_wc/entries.c
>   (svn_wc__temp_mark_missing_not_present): New.
>   (svn_wc_mark_missing_deleted): Call
> svn_wc__temp_mark_missing_not_present.
> 
> * subversion/libsvn_client/commit.c
>   (struct post_commit_baton): Replace access baton member with wc
> context.
>   (post_process_commit_item): Use db functions instead of access baton.
>   (svn_client_commit4): Store wc context instead of access baton.
> 
> Modified:
> subversion/trunk/subversion/include/private/svn_wc_private.h
> subversion/trunk/subversion/include/svn_wc.h
> subversion/trunk/subversion/libsvn_client/commit.c
> subversion/trunk/subversion/libsvn_wc/entries.c
> 
> Modified: subversion/trunk/subversion/include/private/svn_wc_private.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private
> /svn_wc_private.h?rev=921445&r1=921444&r2=921445&view=diff
> ==
> 
> --- subversion/trunk/subversion/include/private/svn_wc_private.h (original)
> +++ subversion/trunk/subversion/include/private/svn_wc_private.h Wed
> Mar 10 16:47:27 2010
> @@ -552,6 +552,18 @@ svn_wc__call_with_write_lock(svn_wc__wit
>   apr_pool_t *result_pool,
>   apr_pool_t *scratch_pool);
> 
> +
> +/** Mark missing, deleted directory @a local_abspath as 'not-present'
> + * in its parent's list of entries.
> + *
> + * Return #SVN_ERR_WC_PATH_FOUND if @a local_abspath isn't actually a
> + * missing, deleted directory.
> + */
> +svn_error_t *
> +svn_wc__temp_mark_missing_not_present(const char *local_abspath,
> +  svn_wc_context_t *wc_ctx,
> +  apr_pool_t *scratch_pool);
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> 
> Modified: subversion/trunk/subversion/include/svn_wc.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc
> .h?rev=921445&r1=921444&r2=921445&view=diff
> ==
> 
> --- subversion/trunk/subversion/include/svn_wc.h (original)
> +++ subversion/trunk/subversion/include/svn_wc.h Wed Mar 10 16:47:27
> 2010
> @@ -3312,7 +3312,10 @@ svn_wc_walk_entries(const char *path,
>  /** Mark missing @a path as 'deleted' in its @a parent's list of entries.
>   *
>   * Return #SVN_ERR_WC_PATH_FOUND if @a path isn't actually missing.
> + *
> + * @deprecated Provided for backward compatibility with the 1.6 API.
>   */
> +SVN_DEPRECATED
>  svn_error_t *
>  svn_wc_mark_missing_deleted(const char *path,
>  svn_wc_adm_access_t *parent,
> 
> Modified: subversion/trunk/subversion/libsvn_client/commit.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/co
> mmit.c?rev=921445&r1=921444&r2=921445&view=diff
> ==
> 
> --- subversion/trunk/subversion/libsvn_client/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_client/commit.c Wed Mar 10
> 16:47:27 2010
> @@ -935,7 +935,7 @@ struct post_commit_baton
>  {
>svn_wc_committed_queue_t *queue;
>apr_pool_t *qpool;
> -  svn_wc_adm_access_t *base_dir_access;
> +  svn_wc_context_t *wc_ctx;
>svn_boolean_t keep_changelists;
>svn_boolean_t keep_locks;
>apr_hash_t *checksums;
> @@ -950,35 +950,23 @@ post_process_commit_item(void *baton, vo
>svn_client_commit_item3_t *item =
>  *(svn_client_commit_item3_t **)this_item;
>svn_boolean_t loop_recurse = FALSE;
> -  const char *adm_access_path;
> -  svn_wc_adm_access_t *adm_access;
>svn_boolean_t remove_lock;
> -  svn_error_t *bump_err;
> -
> -  if (item->kind == svn_node_dir)
> -adm_access_path = item->path;
> -  else
> -adm_access_path = svn_dirent_dirname(item->path, pool);
> 
> -  bump_err = svn_wc_adm_retrieve(&adm_access, btn->base_dir_access,
> - adm_access_path, pool);
> -  if (bump_err
> -  && bump_err->apr_err == SVN_ERR_WC_NOT_LOCKED)
> +  /* Is it a missing, deleted directory?
> +
> + ### Temporary: once we centralise this sort of node is just a
> + normal delete and will get handled by the post-commit queue. */

RE: svn commit: r922176 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_wc/revision_status.c svnversion/main.c

[Bert Huijben]

> -Original Message-
> From: dan...@apache.org [mailto:dan...@apache.org]
> Sent: vrijdag 12 maart 2010 9:22
> To: comm...@subversion.apache.org
> Subject: svn commit: r922176 - in /subversion/trunk/subversion:
> include/svn_wc.h libsvn_wc/revision_status.c svnversion/main.c
> 
> Author: dannas
> Date: Fri Mar 12 08:21:45 2010
> New Revision: 922176
> 
> URL: http://svn.apache.org/viewvc?rev=922176&view=rev
> Log:
> As part of WC-NG, remove some uses of svn_wc_entry_t.
> 
> * subversion/include/svn_wc.h
>   (svn_wc_revision_status2): Add note about us returning
> SVN_ERR_WC_PATH_NOT_FOUND.
> * subversion/libsvn_wc/revision_status.c
>(status_baton): Rename this...
>(walk_baton): .. to this.
>(analyze_status): Change this to implement svn_wc__node_func_t and use
>  WC-NG funcs instead of information in svn_wc_entry_t.
>(svn_wc_revision_status2): Use svn_wc__node_walk_children() instead of
>  svn_wc_walk_status() to spare us from the overhead of invoking an
>  editor.
> * subversion/svnversion/main.c
>   (main): Determine if a path is unversioned by checking for
> SVN_ERR_WC_PATH_NOT_FOUND.
> 
> Approved by: gstein
> 
> Modified:
> subversion/trunk/subversion/include/svn_wc.h
> subversion/trunk/subversion/libsvn_wc/revision_status.c
> subversion/trunk/subversion/svnversion/main.c
> 
> Modified: subversion/trunk/subversion/include/svn_wc.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc
> .h?rev=922176&r1=922175&r2=922176&view=diff
> ==
> 
> --- subversion/trunk/subversion/include/svn_wc.h (original)
> +++ subversion/trunk/subversion/include/svn_wc.h Fri Mar 12 08:21:45 2010
> @@ -7192,7 +7192,8 @@ typedef struct svn_wc_revision_status_t
>  /** Set @a *result_p to point to a new #svn_wc_revision_status_t structure
>   * containing a summary of the revision range and status of the working copy
>   * at @a local_abspath (not including "externals").  @a local_abspath must
> - * be absolute.
> + * be absolute. Return SVN_ERR_WC_PATH_NOT_FOUND if @a
> local_abspath is not
> + * a working copy path.
>   *
>   * Set @a (*result_p)->min_rev and @a (*result_p)->max_rev respectively
> to the
>   * lowest and highest revision numbers in the working copy.  If @a
> committed
> 
> Modified: subversion/trunk/subversion/libsvn_wc/revision_status.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/revis
> ion_status.c?rev=922176&r1=922175&r2=922176&view=diff
> ==
> 
> --- subversion/trunk/subversion/libsvn_wc/revision_status.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/revision_status.c Fri Mar 12
> 08:21:45 2010
> @@ -23,65 +23,100 @@
> 
>  #include "svn_wc.h"
>  #include "svn_dirent_uri.h"
> +#include "wc_db.h"
> +#include "wc.h"
> +#include "props.h"
> 
>  #include "private/svn_wc_private.h"
> 
>  #include "svn_private_config.h"
> 
>  /* A baton for analyze_status(). */
> -struct status_baton
> +struct walk_baton
>  {
>svn_wc_revision_status_t *result;   /* where to put the result */
>svn_boolean_t committed;   /* examine last committed revisions */
>const char *local_abspath; /* path whose URL we're looking for */
> -  const char *wc_url;/* URL for the path whose URL we're looking for */
> -  apr_pool_t *pool; /* pool in which to store alloc-needy things */
> +  svn_wc__db_t *db;
>  };
> 
> -/* An svn_wc_status_func4_t callback function for analyzing status
> -   structures. */
> +/* An svn_wc__node_found_func_t callback function for analyzing the
> status
> + * of nodes */
>  static svn_error_t *
> -analyze_status(void *baton,
> -   const char *local_abspath,
> -   const svn_wc_status2_t *status,
> -   apr_pool_t *pool)
> +analyze_status(const char *local_abspath,
> +   void *baton,
> +   apr_pool_t *scratch_pool)
>  {
> -  struct status_baton *sb = baton;
> +  struct walk_baton *wb = baton;
> +  svn_revnum_t changed_rev;
> +  svn_revnum_t revision;
> +  svn_depth_t depth;
> +  svn_wc__db_status_t status;
> +  svn_boolean_t wc_root;
> +  svn_boolean_t switched;
> +
> +  SVN_ERR(svn_wc__db_read_info(&status, NULL, &revision, NULL,
> +   NULL, NULL, &changed_rev,
> +   NULL, NULL, NULL, &depth, NULL, NULL, NULL,
> +   NULL, NULL, NULL, NULL, NULL, NULL,
> +   NULL, NULL, NULL, NULL, wb->db,
> +   local_abspath, scratch_pool, scratch_pool));
> +
> +  /* We need the excluded and absent paths when checking for sparse
> +   * checkouts. But only for that. To collect those we're walking all the
> +   * hidden nodes. */
> +  if (status == svn_wc__db_status_excluded
> +  || status == svn_wc__db_status_absent

Re: svn commit: r921445 - in /subversion/trunk/subversion: include/private/svn_wc_private.h include/svn_wc.h libsvn_client/commit.c libsvn_wc/entries.c

[Bert Huijben]

On Sun, Mar 14, 2010 at 1:00 AM, Greg Stein  wrote:
> On Fri, Mar 12, 2010 at 06:28, Philip Martin  
> wrote:
>> Greg Stein  writes:
>>
>>> The SUBDIR is a child of DIR in r9, and presumably also r6. When the
>>> commit is performed, constructing r10, SUBDIR is deleted. The working
>>> copy lists DIR at r9 which is *supposed* to include SUBDIR. Yet, due
>>> to mixed-rev working copies, we have SUBDIR at r10 which does not
>>> exist. It is therefore labeled as not-present.
>>
>> I disagree with "we have SUBDIR at r10".  We expect SUBDIR at r9 but
>> the SUBDIR is not-present, its revision doesn't have a meaning.
>>
>>> By r23, it is possible for SUBDIR to return.
>>>
>>> If the parent is updated to r10, then the not-present SUBDIR can be
>>> removed because d...@10 does not list SUBDIR as a child.
>>
>> I think not-present nodes get removed on update irrespective of
>> revision.
>>
>>> The revision is quite necessary.
>>
>> All that you have written just requires SUBDIR not-present, it doesn't
>> require r10.
>>
>> Suppose the commit that deletes SUBDIR is r11 and the parent is r9.
>> When the parent gets updated, be it to r9, r10, r11 or r12 the
>> not-present node gets removed (and possibly replaced by a normal
>> node).  The r11 revision doesn't matter.
>
>  yeah, I think I agree with all this. Thanks for the insight. I
> spent some time pondering on it today, and started to wonder whether
> this applies to "excluded" and "absent" nodes, too (all three are
> basically "the node isn't really there").
>
> However, a node could be absent in a specific revision, yet available
> for other revs. (e.g. administratively hide the one rev which has a
> password in it)
>
> Excluded doesn't really need a revision. "Whatever it is, this node is
> being kept out of the working copy." We can't just wipe them before an
> update (like the not-present nodes) because the concept is sticky.
>
> I'm not entirely sure we should be removing not-present nodes before
> an update (conceptually). There may be a better design that results in
> the same thing, but from a better/cleaner direction.
>
> Lastly... we may want to enforce this notion of "no revision", and
> have base_get_info() and read_info() return SVN_INVALID_REVNUM for
> not-present nodes (and excluded nodes).
>
> Then again, I think of Daniel's work on revision_status() lately. If I do:
>
> $ svn rm path/subdir
> $ svn commit path/subdir
> $ svnversion
>
> Should it report "just r9", or should it report "r9-10" ?

I think it should report r9-10 (as it is not a clean r9), but it most
likely did report just r9 <= 1.6, because it just skipped hidden
entries via the status walker.

update_editor.c's implementation of editor-v1.0 uses the revisions of
not-present nodes as an indicator which can be removed after an update
and which can't. (To support depth limited updating and a few other
things I can't remember without reviewing the code again).

not present nodes are send to the editor reporter as deleted (see
adm_crawler.c) to make them come in as changes if something does exist
there. (This reporting doesn't use a revision). If nothing comes in
(determined as: is the revision not equal to the target revision) the
not present node is removed on a successful close of the directory.

Bert

RE: svn commit: r921445 - in /subversion/trunk/subversion: include/private/svn_wc_private.h include/svn_wc.h libsvn_client/commit.c libsvn_wc/entries.c

[Bert Huijben]

> -Original Message-
> From: Philip Martin [mailto:philip.mar...@wandisco.com]
> Sent: zondag 14 maart 2010 14:38
> To: Greg Stein
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r921445 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h include/svn_wc.h
> libsvn_client/commit.c libsvn_wc/entries.c
> 
> Greg Stein  writes:
> 
> > $ svn rm path/subdir
> > $ svn commit path/subdir
> > $ svnversion
> >
> > Should it report "just r9", or should it report "r9-10" ?
> 
> We could change the definition of r9P to include not-present as well
> as excluded.
> 
> I'd like 'svn status' to start indicating when a working copy is
> sparse, probably by using 'P' like svnversion.  It could also be used
> to indicate not-present nodes.

Where would you like it to report that 'P'. On directories with depth <
svn_depth_infinity?
(Trying to think of a place where that makes sense)

$ svn status
M   subversion\libsvn_wc\update_editor.c
M   subversion\libsvn_wc\wc-queries.sql
M   subversion\libsvn_wc\wc_db.c
M   subversion\libsvn_wc\wc_db.h

Would that indicate a change on a directory? (Otherwise sparse directories
aren't even in the output of svn status)

(Adding yet another column doesn't make things more understandable)

Maybe in svn status -v?

svn info already reports sparse directories, but it currently has no way to
report excluded subdirectories and files. (Which I see as another kind of
sparse directories). And if we think of adding excluded as sparse, something
tells me that we should also look at absent.

Just one thing: One of the major objectives of WC-NG is to make 'svn status'
noticeable faster. Adding more calculations to that might work in the
opposite direction.

Bert

RE: svn commit: r922511 - /subversion/trunk/subversion/libsvn_client/commit.c

[Bert Huijben]

> -Original Message-
> From: phi...@apache.org [mailto:phi...@apache.org]
> Sent: zaterdag 13 maart 2010 10:27
> To: comm...@subversion.apache.org
> Subject: svn commit: r922511 -
> /subversion/trunk/subversion/libsvn_client/commit.c
> 
> Author: philip
> Date: Sat Mar 13 09:27:20 2010
> New Revision: 922511
> 
> URL: http://svn.apache.org/viewvc?rev=922511&view=rev
> Log:
> Remove access batons from client commit code.
> 
> * subversion/libsvn_client/commit.c
>   (struct check_dir_delete_baton): Remove access baton.
>   (check_nonrecursive_dir_delete): Check locks instead of access
> batons.
>   (svn_client_commit4): Acquire and release locks instead of access
> batons.

Since this patch the fsfs x svn buildbot reports:
FAIL:  commit_tests.py 27: committing two WCs from different repos fails
(The other RA layers don't)

CMD: svn commit -m log svn-test-work/working_copies/commit_tests-27/wc1 
svn-test-work/working_copies/commit_tests-27/wc2 --config-dir 
/home/svnbuilder/slave/eh-debsarge1/build/subversion/tests/cmdline/svn-test-work/local_tmp/config
 --password rayjandom --no-auth-cache --username jrandom
CMD: /home/svnbuilder/slave/eh-debsarge1/build/subversion/svn/svn commit -m log 
svn-test-work/working_copies/commit_tests-27/wc1 
svn-test-work/working_copies/commit_tests-27/wc2 --config-dir 
/home/svnbuilder/slave/eh-debsarge1/build/subversion/tests/cmdline/svn-test-work/local_tmp/config
 --password rayjandom --no-auth-cache --username jrandom exited with 1

subversion/svn/commit-cmd.c:142: (apr_err=155004)
subversion/libsvn_client/commit.c:1172: (apr_err=155004)
subversion/libsvn_client/commit.c:1028: (apr_err=155004)
svn: Are all targets part of the same working copy?
svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
Expected output on stderr doesn't match
EXPECTED STDERR (regexp):
.*is not a working copy.*
ACTUAL STDERR:
subversion/svn/commit-cmd.c:142: (apr_err=155004)
subversion/libsvn_client/commit.c:1172: (apr_err=155004)
subversion/libsvn_client/commit.c:1028: (apr_err=155004)
svn: Are all targets part of the same working copy?
svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
EXCEPTION: SVNUnmatchedError

Bert 

RE: svn commit: r922511 - /subversion/trunk/subversion/libsvn_client/commit.c

[Bert Huijben]

> -Original Message-
> From: Philip Martin [mailto:philip.mar...@wandisco.com]
> Sent: zondag 14 maart 2010 19:10
> To: Bert Huijben
> Cc: dev@subversion.apache.org; phi...@apache.org
> Subject: Re: svn commit: r922511 -
> /subversion/trunk/subversion/libsvn_client/commit.c
> 
> "Bert Huijben"  writes:
> 
> >> New Revision: 922511
> >>
> >> URL: http://svn.apache.org/viewvc?rev=922511&view=rev
> >> Log:
> >> Remove access batons from client commit code.
> >>
> >> * subversion/libsvn_client/commit.c
> >>   (struct check_dir_delete_baton): Remove access baton.
> >>   (check_nonrecursive_dir_delete): Check locks instead of access
> >> batons.
> >>   (svn_client_commit4): Acquire and release locks instead of access
> >> batons.
> >
> > Since this patch the fsfs x svn buildbot reports:
> > FAIL:  commit_tests.py 27: committing two WCs from different repos
> fails
> > (The other RA layers don't)
> 
> It works for me.  r922867 was dodgy, but r922926 should have fixed it.

http://buildbot.subversion.org/buildbot/waterfall still says that the svn x
fsfs build testsuite is broken.
(This buildbot uses a recent trunk for checking out the build working
copy... that could make a difference for the specific test)

Bert

RE: wc-ng base/working nodes in a copied tree

[Bert Huijben]

> -Original Message-
> From: Philip Martin [mailto:philip.mar...@wandisco.com]
> Sent: maandag 22 maart 2010 23:00
> To: Greg Stein
> Cc: dev@subversion.apache.org
> Subject: Re: wc-ng base/working nodes in a copied tree
> 
> Greg Stein  writes:
> 
> > The tree at copied-here should only have WORKING nodes. No BASE
> nodes.
> >
> > If it has BASE nodes, then that is a bug.
> >
> > The tree is distinguished as a copy because of the copyfrom_*
> > information at the operation root. All the children have empty
> > copyfrom_* data. If you make a second copy into that tree, then that
> > new subtree will have copyfrom_* at its root.
> 
> My question was about an add rather than a second copy.  Consider
> 
>$ svn cp $url/A wc
>$ svn add wc/A/Y
> 
> Suppose $url/A contains $url/A/X.  How do I distinguish between a
> copied child, like wc/A/X, and an added node like wc/A/Y?  Neither has
> copyfrom set.  How do I know that A/X inherits from it's parent A
> while A/Y does not?

There is no clean way to determine this, which should be fixed by making it
explicit (how is still undetermined)

For the time being you can see a difference in the changed_* columns. Local
additions have no (=NULL) values here, while copies have at least a
changed_rev value. (This is how the entries read code currently determines
whether it should set .copied or not)

Bert

RE: Why merge is so slow? (was Re: svn commit: r926210 - /subversion/trunk/notes/meetings/svn-vision-agenda)

[Bert Huijben]

> -Original Message-
> From: Stefan Sperling [mailto:s...@elego.de]
> Sent: dinsdag 23 maart 2010 14:15
> To: Mark Phippard
> Cc: Johan Corveleyn; Ivan Zhakov; hwri...@apache.org;
> dev@subversion.apache.org
> Subject: Re: Why merge is so slow? (was Re: svn commit: r926210 -
> /subversion/trunk/notes/meetings/svn-vision-agenda)
> 
> On Tue, Mar 23, 2010 at 08:32:32AM -0400, Mark Phippard wrote:
> > On Tue, Mar 23, 2010 at 8:28 AM, Stefan Sperling  wrote:
> >
> > > In most setups I've seen the server hardware is much beefier than
> > > the client hardware, so unless we do things that scale really badly
> > > (say more than O(n^2)) I don't see a problem.
> >
> > Think of a hosting site like sf.net with thousands of SVN repos being
> > hit by many thousands of users.  How many of these operations do you
> > think the Apache server could manage before it ran out of RAM?
> 
> If such sites run a single server only and don't use write-through
> proxies to balance the load their setup is seriously wrong.
> 
> And I'd say users will happily accept more load on the server if that
> means that they get working renames in return. You can throw more
> machines at the performance problem, but not at the rename problem.

Handling true renames is a completely different issue then this performance
issue and/or editor v2. We want all three issues to be resolved, but the
issues don't depend on each other.

WC-NG makes the working copy ready for recording moves, but what is still
missing is support of true moves in the repository and filesystem.


If we don't want to change the editor just now we could just use the 'entry
property' infrastructure to communicate the information that a specific copy
is actually a move. (This would fix the cases of moving files and doesn't
require any editor or implementation fixes.  Directory moves are not
communicated as copies over the update editor in the current editorV1 code,
so that would require a separate fix. But we can easily work around this
using the capability negotiation we added in 1.5).


But then we need to update merge itself to handle the renames. This is most
likely the biggest chunk of work, but I'm not a merge expert.

Bert

Constifying patches break bindings

[Bert Huijben]

Posted to dev@, in an attempt to get the buildbots green again:

 

(Yesterday evening; times in CET / UTC+1)

 

22:20 < CIA-77> rdonch * r926343 /trunk/subversion/bindings/swig/svn_diff.i:
* subversion/bindings/swig/svn_diff.i:

Immutablize some struct members, ...

22:27 <@Bert> RDonch: Does this fix the buildbots?

22:28 < RDonch> Bert: almost.



22:31 <@Bert> RDonch: It fails on the x64-ubuntu-gcc bot.. But it looks like
the centos bot gets further than before

22:31 < RDonch> julianf: in r922239 you add a bunch of const qualifiers and
say that it's a backwards-compatible change

- but is it? E.g. a function that was a
svn_ra_file_rev_handler_t will no longer be, causing compiler

errors.

22:31 < RDonch> Bert: ^ this is the remaining failure.

 

 

RE: switching added files -- Re: What revision should an added not yet commited node have?

[Bert Huijben]

> -Original Message-
> From: neels [mailto:nee...@gmail.com]
> Sent: dinsdag 23 maart 2010 19:13
> To: Greg Stein
> Cc: Julian Foad; dev@subversion.apache.org
> Subject: Re: switching added files -- Re: What revision should an added
> not yet commited node have?
> 
> On 22 March 2010 17:20, Greg Stein  wrote:
> > I think the child is *always* a child of the parent.
> 
> Greg, you definitely know that this works today:
> 
> [[[
> $ svn add bar/y
> A bar/y
> 
> $ svn ci -mm
> Adding bar/y
> 
> $ svn switch --depth=empty '^/foo' bar
> At revision 1.

Note that while this might be/look supported... it is also a typical example
which triggers issue #3569.

You switch the directory to a new revision, but don't update its list of
children to match that revision. So you will never receive the children of
the node, even if you perform svn update --set-depth infinity.

(There is no way to fix this situation except switching this node back to
its original locat...@rev with depth empty and then performing the switch
recursively)

Bert

RE: svn commit: r926613 - in /subversion/trunk/build/generator: gen_win.py templates/vcnet_vcproj.ezt templates/vcnet_vcxproj.ezt

[Bert Huijben]

> -Original Message-
> From: Greg Stein [mailto:gst...@gmail.com]
> Sent: dinsdag 23 maart 2010 22:00
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r926613 - in
> /subversion/trunk/build/generator: gen_win.py
> templates/vcnet_vcproj.ezt templates/vcnet_vcxproj.ezt
> 
> On Tue, Mar 23, 2010 at 11:25,   wrote:
> >...
> > +++ subversion/trunk/build/generator/templates/vcnet_vcproj.ezt Tue
> Mar 23 15:25:10 2010
> > @@ -128,7 +128,8 @@
> >                                 >                                        Name="VCCustomBuildTool"
> >
>  CommandLine="[sources.custom_build]"
> > -                                       AdditionalDependencies="[for
> sources.user_deps]"[sources.user_deps]";[end]"
> > +[is sources.custom_desc ""][else]
>    Description="[sources.custom_desc]"
> > +[end]                                  AdditionalDependencies="[for
> sources.user_deps]"[sources.user_deps]";[end]"
> 
> That's an ugly construction. Just use:
> 
> [if-any sources.custom_desc]...[end]

Thanks, r926847.

Bert

RE: svn commit: r926814 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c update_editor.c workqueue.c

[Bert Huijben]

> -Original Message-
> From: gst...@apache.org [mailto:gst...@apache.org]
> Sent: dinsdag 23 maart 2010 23:16
> To: comm...@subversion.apache.org
> Subject: svn commit: r926814 - in
> /subversion/trunk/subversion/libsvn_wc: adm_ops.c update_editor.c
> workqueue.c
> 
> Author: gstein
> Date: Tue Mar 23 22:16:21 2010
> New Revision: 926814
> 
> URL: http://svn.apache.org/viewvc?rev=926814&view=rev
> Log:
> Use the new file installation work item within svn_wc_add_repos_file4()
> 
> * subversion/libsvn_wc/update_editor.c:
>   (svn_wc_add_repos_file4): remove computation of DST_TXTB since we
> already have it as TEXT_BASE_PATH. add comment about possibly using
> OP_FILE_INSTALL when a separate working file (as distinct from just
> the text base) is provided. remove the old section about installing
> from the base, and do it later as a work item. add a comment to
> clarify what svn_wc__install_props() is doing w.r.t work items.
> only
> do an (extra) run of the workqueue before we make a call to
> db_temp_op_set_working_last_change; otherwise, we can wait and run
> everything all at once.
> 
> * subversion/libsvn_wc/adm_ops.c:
>   (svn_wc__get_pristine_contents): added nodes *do* have pristines
> 
> * subversion/libsvn_wc/workqueue.c:
>   (run_file_install): assert that we got a pristine stream (this work
> item
> should not have been used, otherwise!)

Hi Greg,

This patch breaks export test 11 on more than one buildbot:

FAIL:  export_tests.py 11: export working copy at base revision

CMD: svn export svn-test-work/working_copies/export_tests-11 
svn-test-work/working_copies/export_tests-11.export -rBASE --config-dir 
/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/svn-test-work/local_tmp/config
 --password rayjandom --no-auth-cache --username jrandom
CMD: /home/bt/slaves/x64-centos/build/subversion/svn/svn export 
svn-test-work/working_copies/export_tests-11 
svn-test-work/working_copies/export_tests-11.export -rBASE --config-dir 
/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/svn-test-work/local_tmp/config
 --password rayjandom --no-auth-cache --username jrandom exited with 1

subversion/svn/export-cmd.c:104: (apr_err=2)
subversion/libsvn_client/export.c:1080: (apr_err=2)
subversion/libsvn_client/export.c:426: (apr_err=2)
subversion/libsvn_client/export.c:164: (apr_err=2)
subversion/libsvn_subr/stream.c:745: (apr_err=2)
subversion/libsvn_subr/stream.c:745: (apr_err=2)
subversion/libsvn_subr/stream.c:745: (apr_err=2)
subversion/libsvn_subr/io.c:2696: (apr_err=2)
svn: Can't open file 
'/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/svn-test-work/working_copies/export_tests-11/.svn/text-base/kappa.svn-base':
 No such file or directory
Traceback (most recent call last):
  File 
"/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/svntest/main.py", 
line 1197, in run
rc = self.pred.run(sandbox)
  File 
"/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/svntest/testcase.py",
 line 160, in run
return self.func(sandbox)
  File 
"/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/export_tests.py", 
line 293, in export_working_copy_at_base_revision
'-rBASE')
  File 
"/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/svntest/actions.py", 
line 390, in run_and_verify_export
URL, export_dir_name, *args)
  File 
"/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/svntest/main.py", 
line 604, in run_svn
return run_command(svn_binary, error_expected, 0,
  File 
"/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/svntest/main.py", 
line 366, in run_command
None, *varargs)
  File 
"/home/bt/slaves/x64-centos/build/subversion/tests/cmdline/svntest/main.py", 
line 538, in run_command_stdin
raise Failure
Failure

Bert

RE: Hook scripts start with an empty environment

[Bert Huijben]

> -Original Message-
> From: Tim Starling [mailto:tstarl...@wikimedia.org]
> Sent: woensdag 24 maart 2010 7:19
> To: dev@subversion.apache.org
> Subject: Hook scripts start with an empty environment
> 
> Hook scripts start with an empty environment instead of inheriting it
> from svnserve or whatever.
> 
> This is inconvenient, not least for the case where you want to commit
> something to an svn+ssh server via a local pushmi mirror on file:/// and
> your SSH_AUTH_SOCK is lost so you have to type your passphrase all the
> time.
> 
> There's no comment in the code explaining why the environment has to be
> empty, so I assume it was just done like that on a whim. Trivial patch
> attached.

Sorry,

This behavior is by design. 

Repository hooks run as the 'repository owner' and clearing the environment is 
part of the security around that feature.
http://svnbook.red-bean.com/en/1.5/svn.reposadmin.create.html#svn.reposadmin.create.hooks
 (or http://tinyurl.com/59yzll )

I'm a bit surprised that you actually see a passphrase prompt from a hook, as 
the hook environment redirects stdin, stdout and stderr to the server process. 
The only prompt you should be able to see is the prompt for starting the ssh 
process. (And this ssh isn't called via the function you tried to patch)

If we would forward the environment hook scripts, the scripts might 
accidentally use environment variables from the calling process without the 
user knowing. Which opens a backdoor for all kinds of malware/abusal. And it 
would also make it very hard to create hook scripts that work identical for all 
repository users.

Bert

RE: svn commit: r927620 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/patch.c svn/patch-cmd.c tests/libsvn_client/ tests/libsvn_client/client-test.c

[Bert Huijben]

> -Original Message-
> From: s...@apache.org [mailto:s...@apache.org]
> Sent: donderdag 25 maart 2010 23:44
> To: comm...@subversion.apache.org
> Subject: svn commit: r927620 - in /subversion/trunk/subversion:
> include/svn_client.h libsvn_client/patch.c svn/patch-cmd.c
> tests/libsvn_client/ tests/libsvn_client/client-test.c
> 
> Author: stsp
> Date: Thu Mar 25 22:44:06 2010
> New Revision: 927620
> 
> URL: http://svn.apache.org/viewvc?rev=927620&view=rev
> Log:
> Fix issue #3598, "Allow access to patched temporary files via svn patch API"
> 
> * subversion/include/svn_client.h
>   (svn_client_patch): Add patched_tempfiles and reject_tempfiles output
>parameters.  Switch this function to dual-pool style since it now
>has output parameters.
> 
> * subversion/libsvn_client/patch.c
>   (init_patch_target, apply_one_patch, svn_client_patch): Add
>patched_tempfiles and reject_tempfiles parameters. If provided,
>put paths to temporary files into them.
>   (apply_patches_baton_t): Add patched_tempfiles and reject_tempfiles
>member fields.
> 
> * subversion/svn/patch-cmd.c
>  (svn_cl__patch): Track parameter changes to svn_client_patch().
> 
> * subversion/tests/libsvn_client: Ignore test-patch*
> 
> * subversion/tests/libsvn_client/client-test.c
>   (): Include some headers.
>   (check_patch_result): Helper for new test.
>   (test_patch): New test which checks whether tempfiles are returned
>properly, are not deleted, and have expected content. We cannot test
>this feature via cmdline tests because the CLI client does not expose it.
>   (test_funcs): Add new test.
> 
> Modified:
> subversion/trunk/subversion/include/svn_client.h
> subversion/trunk/subversion/libsvn_client/patch.c
> subversion/trunk/subversion/svn/patch-cmd.c
> subversion/trunk/subversion/tests/libsvn_client/   (props changed)
> subversion/trunk/subversion/tests/libsvn_client/client-test.c
> 



> Modified: subversion/trunk/subversion/tests/libsvn_client/client-test.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_cli
> ent/client-test.c?rev=927620&r1=927619&r2=927620&view=diff
> ==
> 
> --- subversion/trunk/subversion/tests/libsvn_client/client-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_client/client-test.c Thu Mar
> 25 22:44:06 2010
> @@ -22,12 +22,17 @@
>   */
> 
>  

> +#include 

This is a unix only include file. 
Please use some appropriate check (E.g. APR_HAVE_UNISTD_H) or just use an apr 
equivalent.
(Fixed in r 927764)

> +#include 
>  #include "svn_mergeinfo.h"
>  #include "../../libsvn_client/mergeinfo.h"
>  #include "svn_pools.h"
>  #include "svn_client.h"
> +#include "svn_repos.h"
> +#include "svn_subst.h"
> 
>  #include "../svn_test.h"
> +#include "../svn_test_fs.h"
> 
>  typedef struct {
>const char *path;
> @@ -199,6 +204,163 @@ test_args_to_target_array(apr_pool_t *po
>return SVN_NO_ERROR;
>  }
> 
> +
> +/* A helper function for test_patch().
> + * It compares a patched or reject file against expected content.
> + * It also deletes the file if the check was successful. */
> +static svn_error_t *
> +check_patch_result(const char *path, const char **expected_lines,
> +   int num_expected_lines, apr_pool_t *pool)
> +{
> +  svn_string_t *path_svnstr;
> +  svn_string_t *path_utf8;
> +  apr_file_t *patched_file;
> +  svn_stream_t *stream;
> +  apr_pool_t *iterpool;
> +  int i;
> +
> +  path_svnstr = svn_string_create(path, pool);
> +  SVN_ERR(svn_subst_translate_string(&path_utf8, path_svnstr, NULL,
> pool));
> +  SVN_ERR(svn_io_file_open(&patched_file, path_utf8->data,
> +   APR_READ, APR_OS_DEFAULT, pool));
> +  stream = svn_stream_from_aprfile2(patched_file, TRUE, pool);
> +  i = 0;
> +  iterpool = svn_pool_create(pool);
> +  while (TRUE)
> +{
> +  svn_boolean_t eof;
> +  svn_stringbuf_t *line;
> +
> +  svn_pool_clear(iterpool);
> +
> +  SVN_ERR(svn_stream_readline(stream, &line, APR_EOL_STR, &eof,
> pool));
> +  if (i < num_expected_lines)
> +SVN_ERR_ASSERT(strcmp(expected_lines[i++], line->data) == 0);
> +  if (eof)
> +break;
> +}
> +  svn_pool_destroy(iterpool);
> +
> +  SVN_ERR(svn_io_file_close(patched_file, pool));
> +  SVN_ERR_ASSERT(i == num_expected_lines);
> +  SVN_ERR(svn_io_remove_file2(path_utf8->data, FALSE, pool));
> +
> +  return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *
> +test_patch(const svn_test_opts_t *opts,
> +   apr_pool_t *pool)
> +{
> +  svn_repos_t *repos;
> +  svn_fs_t *fs;
> +  svn_fs_txn_t *txn;
> +  svn_fs_root_t *txn_root;
> +  apr_hash_t *patched_tempfiles;
> +  apr_hash_t *reject_tempfiles;
> +  const char *repos_url;
> +  const char *wc_path;
> +  char cwd[PATH_MAX];

You don't want to use PATH_MAX on Windows. It is about 256 there, but APR has 
no problems handling paths that are much longer. APR_PATH_MAX i

RE: [PATCH] delta_files() speedup 3/3: file write buffering

[Bert Huijben]

[CC to d...@apr.apache.org]

> -Original Message-
> From: Stefan Fuhrmann [mailto:stefanfuhrm...@alice-dsl.de]
> Sent: zondag 28 maart 2010 14:51
> To: dev@subversion.apache.org
> Subject: [PATCH] delta_files() speedup 3/3: file write buffering
> 
> Hi devs,
> 
> this is part of the delta_files() speedup patch series:
> http://svn.haxx.se/dev/archive-2010-03/0604.shtml
> 
> Under Windows, a significant part of the overall runtime
> is spent in writing data to the (already buffered) APR
> file implementation. It seems that the mutex serializing
> the buffer access in apr_file_write is expensive.
> 
> Also, >50% of all write requests are 2 bytes or smaller
> (i.e. line endings and empty lines). For them, the deep
> call hierarchy constitutes a large overhead on register-
> lacking x86.
> 
> This patch eliminates far over 90% of all write requests
> bringing the portion of time spent in _svn_io_file_write_full
> down from about 7 to below 3 percent on 32 bit Windows.
> 
> Performance gain is ~1% under Linux but due to the
> larger async I/O and mutex overhead it is about 4%
> under Windows:

Hi Stefan,

Thanks for looking into this.

I'm just wondering why APR does use the mutexes even though APR_XTHREAD
isn't passed?
Maybe because the mutex APR uses, a critical section, is more costly now
than when this code was implemented several years ago? 
(On a singlecore machine a simple interlocked increment is very cheap, but
on a multicore machine it requires at least some synchronization).

Looking at the APR help it might be better to fix the buffering in APR, than
to add yet another level of buffering. (Our buffer, APR's buffering and then
the OS buffering).

After a quick search through the apr code it seems that only windows tries
to make file writes always thread safe, while other operating systems use
APR_XTHREAD as a trigger.


The Windows code also enables overlapped IO on APR_XTHREAD, so there could
be some valid use where you want thread safe code, but not overlapped. But
why only do this on Windows? (Maybe the APR team can answer this question).

Bert

> 
> s~$ time ~/1.7-928181/svn export --ignore-externals -q $REPO/trunk
> /dev/shm/t
> real0m3.727s
> user0m3.189s
> sys 0m0.542s
> 
> ~$ time ~/1.7-patched/svn export --ignore-externals -q $REPO/trunk
> /dev/shm/t
> real0m3.697s
> user0m3.148s
> sys 0m0.558s
> 
> -- Stefan^2.
> 
> [[[
> Buffer small write requests to APR file streams. For details see
> http:// ...
> 
> * subversion/libsvn_subr/stream.c
>   (baton_apr): add write buffer members
>   (flush_buffer_apr): new function
>   (read_handler_apr): auto-allocate write buffer and re-direct
>small writes there.
>   (read_handler_apr, close_handler_apr, mark_handler_apr,
>seek_handler_apr): flush write buffer before calling i/o functions.
> 
> patch by stefanfuhrmann < at > alice-dsl.de
> ]]]

RE: [PATCH] delta_files() speedup 2/3: keyword substitution

[Bert Huijben]

> -Original Message-
> From: Philip Martin [mailto:philip.mar...@wandisco.com]
> Sent: maandag 29 maart 2010 18:20
> To: Julian Foad
> Cc: Stefan Fuhrmann; dev@subversion.apache.org
> Subject: Re: [PATCH] delta_files() speedup 2/3: keyword substitution
> 
> Julian Foad  writes:
> 
> >> * subversion/libsvn_subr/subst.c
> >>   (translation_baton): the 'interesting' member is now
> >>   a boolean array.
> >>   (create_translation_baton): adapt initialization code
> >>   (translate_chunk): eliminate call to strchr
> >>
> >> patch by stefanfuhrmann < at > alice-dsl.de
> >> ]]]
> >
> > This patch looks lovely, from the point of view of a read-through
> > review.
> 
> Agreed.
> 
> To get rid of the initialization we could use 4 static constant arrays
> (we could even partially overlap them to save memory), but that's
> probably not a significant improvement.

I'm not sure how all this compares to just three byte compares, but with a
only a few kb first level cache in most current x86 processors it might be
even more optimal to just do the comparison in code. 

But I think any solution that avoids calling the locale dependent strchr()
function will help here and the details between the table and in-code
variants are probably not measurable.

Bert

RE: allowing multiple conflicts in storage (Re: svn commit: r928806 - /subversion/trunk/notes/wc-ng/conflict-storage)

[Bert Huijben]

> -Original Message-
> From: Stefan Sperling [mailto:s...@elego.de]
> Sent: dinsdag 30 maart 2010 16:30
> To: dev@subversion.apache.org
> Cc: comm...@subversion.apache.org
> Subject: allowing multiple conflicts in storage (Re: svn commit: r928806 -
> /subversion/trunk/notes/wc-ng/conflict-storage)
> 
> On Mon, Mar 29, 2010 at 03:37:51PM -, rhuij...@apache.org wrote:
> > Author: rhuijben
> > Date: Mon Mar 29 15:37:50 2010
> > New Revision: 928806
> >
> > URL: http://svn.apache.org/viewvc?rev=928806&view=rev
> > Log:
> > * notes/wc-ng/conflict-storage
> >   Update schema to have some global information that applies to all
> >   conflict data. Remove this data from the individual locations as it
> >   can't differ between these locations anyway (or the node would have
> >   been skipped). Finally add unversioned obstructions as a conflict
> >   class.
> 
> > -Text conflicts
> > ---
> > +Common conflict data
> > +
> >
> > -Text conflicts only exist on files. The following information
> > -is stored if there is a text conflict on the node:
> > +Some information is shared for all conflict data that applies to a
node. E.g.
> > +when a node has a combination of text and property conflicts these were
> > +always caused by the same operation. (Any later operation will skip the
> node
> > +unless the conflicts are resolved)
> >
> 
> I'm still not sure that I agree with this.
> 
> I don't think it's smart to restrict the conflict storage layer
> to accept only a single set of conflicts for a node, all of which
> belong together to the point where you cannot do anything to the
> node unless you have resolved *all* conflicts.
> 
> I'd like the storage layer to allow for resolving each type of conflicts
> separately from the other types. Higher-level operations can then decide
> whether it is safe to do something to the partially-conflicted node.
> E.g. if a file has only property conflicts, why should a merge skip
> merging changes into the file itself? If a directory is tree conflicted,
> why should I not be allowed to merge some content into a file below
> the tree-conflicted directory if doing so helps me to resolve the tree
> conflict?

I intended to allow resolving only some parts of the conflict; but we can't
allow operations that can introduce conflicts on their own.

You can't apply a merge/update/switch/patch on a node that has a
text/property/obstruction/patch conflict, because a can cause a conflict
with the conflict state (How would you describe that?). You can't trust the
in-wc state if you have a conflict.

In 1.6 the way to resolve a text conflict is to edit the in-wc text with all
the information provided and then mark it resolved.

The same applies to properties: You read the '.prej' file and decide what
the new property value(s) must be and you mark all resolved. (Or just one
with the new storage).


But that doesn't make it possible to introduce new conflicts on top of
existing conflicts; that is +- impossible. 

Just looking at text conflicts: merging will fail because you see conflict
markers.
Same applies to property merges: You can't merge a single property change
into the WC. (Would you use left, right or mine?)

Patch conflicts... see text conflicts.

Tree conflicts: Do you apply the change on BASE_NODE, or on WORKING_NODE
when the node is in a tree conflict?

Obstructions: You can't do anything to the in-wc data

What do we have left?


This is exactly why you need interactive conflict resolution while merging:
That is the only way you can continue a compound operation.


Bert

RE: allowing multiple conflicts in storage (Re: svn commit: r928806 - /subversion/trunk/notes/wc-ng/conflict-storage)

[Bert Huijben]

> -Original Message-
> From: Stefan Sperling [mailto:s...@elego.de]
> Sent: dinsdag 30 maart 2010 18:15
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: allowing multiple conflicts in storage (Re: svn commit:
> r928806 - /subversion/trunk/notes/wc-ng/conflict-storage)
> 
> On Tue, Mar 30, 2010 at 05:55:00PM +0200, Bert Huijben wrote:
> >
> >
> > > -Original Message-
> > > From: Stefan Sperling [mailto:s...@elego.de]
> > > Sent: dinsdag 30 maart 2010 16:30
> > > To: dev@subversion.apache.org
> > > Cc: comm...@subversion.apache.org
> > > Subject: allowing multiple conflicts in storage (Re: svn commit:
> r928806 -
> > > /subversion/trunk/notes/wc-ng/conflict-storage)
> > >
> > > On Mon, Mar 29, 2010 at 03:37:51PM -, rhuij...@apache.org
> wrote:
> > > > Author: rhuijben
> > > > Date: Mon Mar 29 15:37:50 2010
> > > > New Revision: 928806
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=928806&view=rev
> > > > Log:
> > > > * notes/wc-ng/conflict-storage
> > > >   Update schema to have some global information that applies to
> all
> > > >   conflict data. Remove this data from the individual locations
> as it
> > > >   can't differ between these locations anyway (or the node would
> have
> > > >   been skipped). Finally add unversioned obstructions as a
> conflict
> > > >   class.
> > >
> > > > -Text conflicts
> > > > ---
> > > > +Common conflict data
> > > > +
> > > >
> > > > -Text conflicts only exist on files. The following information
> > > > -is stored if there is a text conflict on the node:
> > > > +Some information is shared for all conflict data that applies to
> a
> > node. E.g.
> > > > +when a node has a combination of text and property conflicts
> these were
> > > > +always caused by the same operation. (Any later operation will
> skip the
> > > node
> > > > +unless the conflicts are resolved)
> > > >
> > >
> > > I'm still not sure that I agree with this.
> > >
> > > I don't think it's smart to restrict the conflict storage layer
> > > to accept only a single set of conflicts for a node, all of which
> > > belong together to the point where you cannot do anything to the
> > > node unless you have resolved *all* conflicts.
> > >
> > > I'd like the storage layer to allow for resolving each type of
> conflicts
> > > separately from the other types. Higher-level operations can then
> decide
> > > whether it is safe to do something to the partially-conflicted
> node.
> > > E.g. if a file has only property conflicts, why should a merge skip
> > > merging changes into the file itself? If a directory is tree
> conflicted,
> > > why should I not be allowed to merge some content into a file below
> > > the tree-conflicted directory if doing so helps me to resolve the
> tree
> > > conflict?
> >
> > I intended to allow resolving only some parts of the conflict; but we
> can't
> > allow operations that can introduce conflicts on their own.
> 
> Sure.
> 
> > You can't apply a merge/update/switch/patch on a node that has a
> > text/property/obstruction/patch conflict, because a can cause a
> conflict
> > with the conflict state (How would you describe that?). You can't
> trust the
> > in-wc state if you have a conflict.
> >
> > In 1.6 the way to resolve a text conflict is to edit the in-wc text
> with all
> > the information provided and then mark it resolved.
> 
> Yes, and it's a pain that I can't just do:
> 
>   svn merge ^/blah/fo...@2 ^/blah/fo...@3 bar.c
> 
> as part of my local edits.
> 
> And (speaking from a user perspective who doesn't know how svn works
> internally
> for a minute) why doesn't svn let me do this merge if bar.c is a tree
> conflict victim, but has not text conflicts recorded on it?
> 
> I know that it may be impossible for us to figure out what the merge
> really does before we carry out the merge. We might not know if the
> merge is safe. But why design the storage layer around this assumption?
> It's easy enough to enforce the same restriction at a higher layer.
> 
> > The same applies to properties: You read the '.prej' file and decide
> what
> > the new property value(s) must b

RE: redundant path functions

[Bert Huijben]

> -Original Message-
> From: Greg Stein [mailto:gst...@gmail.com]
> Sent: woensdag 31 maart 2010 2:47
> To: dev@subversion.apache.org
> Subject: redundant path functions
> 
> The following functions seem very redundant. Because each is slightly
> different, I always have to compare/contrast them to isolate their
> differences. It would be much better if we could pick JUST ONE, and
> run with that:
> 
> svn_*_is_child()
> svn_*_is_ancestor()
> svn_*_skip_ancestor()
> 
> I do realize that we published the svn_dirent_* functions in 1.6, but
> we don't have to carry their analogues to the new functions. (and
> possibly even deprecate the 1.6 funcs)
> 
> It seems that the is_child variant is the only function needed. The
> is_ancestor() is merely is_child() != NULL, and the skip_ancestor is
> simply pool=NULL.
> 
> Am I missing something, and/or can/should we remove this redundancy?

There is one difference between is_child and the ancestors functions: How
they handle the case where the path is the ancestor itself. 

The is_child function returns NULL, while is_ancestor returns true.
Skip ancestor removes the ancestor (prefix) and returns "" for the ancestor
itself and leaves paths that don't have the ancestor as-is.

Bert

RE: wc-ng base/working nodes in a copied tree

[Bert Huijben]

> -Original Message-
> From: Philip Martin [mailto:philip.mar...@wandisco.com]
> Sent: woensdag 31 maart 2010 11:20
> To: Greg Stein
> Cc: neels; dev@subversion.apache.org
> Subject: Re: wc-ng base/working nodes in a copied tree
> 
> Greg Stein  writes:
> 
> > On Wed, Mar 24, 2010 at 16:41, Philip Martin
>  wrote:
> >> neels  writes:
> >>> On 23 March 2010 09:11, Greg Stein  wrote:
>  On Mon, Mar 22, 2010 at 17:59, Philip Martin
>  wrote:
> >>
> >> We should consider using copyfrom_repos_path.  The current method of
> >> only storing copyfrom_* on the root of the copy means that
> >> copyfrom_repos_path needs to be calculated every time its value is
> >> required.
> >
> > I doubt that we use it independently of the other fields, so scanning
> > upwards for the others can also compute the relpath.
> >
> > We do the same thing for the regular repos_id and repos_relpath.
> 
> I see there is a comment to that effect in wc-metadata.sql so perhaps
> that was the intent, but in practice repos_id and repos_relpath appear
> to be set in every base_node.

This is mostly caused by the write from entry code, which will disappear before 
1.7. 

But I expect that we will find quite some code that relies on the data being 
available anyway. One thing that certainly relies on this is the retrieval of 
locks via a JOIN with the locks table.

Bert

RE: allowing multiple conflicts in storage

[Bert Huijben]

> -Original Message-
> From: Stefan Sperling [mailto:s...@elego.de]
> Sent: dinsdag 30 maart 2010 19:34
> To: Julian Foad
> Cc: Bert Huijben; dev@subversion.apache.org
> Subject: Re: allowing multiple conflicts in storage
> 
> On Tue, Mar 30, 2010 at 05:56:54PM +0100, Julian Foad wrote:
> >
> > > Conflict meta data storage in wc-ng
> > > ===
> > >
> > > Conflict meta data is stored in the ACTUAL_NODE table, within the
> > > 'conflict_data' column. The data in this column is a skel containing
> > > conflict information, or NULL (meaning no conflict is present).
> >
> > Is storing a record in ACTUAL OK even for a node that (apart from the
> > conflict descripition) has no BASE and no ACTUAL in the WC?  E.g.
> > because it wasn't there and an incoming delete cause a tree conflict.
> > What values would the rest of the ACTUAL_NODE columns have in this
> case?
> 
> Not sure right now. I guess I don't know the schema well enough yet to
> answer this question. Anyone?

All columns in ACTUAL (except wc_id, local_relpath and parent_relpath) are
optional (default=NULL), so this is not an issue.

You have exactly the same thing when you would apply a changelist on a path
and then remove it.

The only special case is that you can have tree conflicts on nodes that are
neither in WORKING nor in BASE. (That is why we stored them on the parent
directory in 1.6)

We can just allow that case in WC-NG, by enumerating over the nodes in
ACTUAL in svn_wc__db_read_conflict_victims(), while
svn_wc__db_read_children() doesn't look at ACTUAL.

(svn_wc__db_read_conflict_victims() currently fakes this behavior by reading
ACTUAL and the parents ACTUAL, parsing the old conflict data)

Bert

Working copies on Windows FAT partitions (E.g. usb sticks) broken in 1.6.10, or

[Bert Huijben]

See ^/subversion/branches/1.6.x/STATUS:

* r908980, r908981
   This followup on r896915, makes sure that the Windows specific remove
   readonly attribute code still works on FAT32 filesystems. (The FAT32
   driver reports a very different error than the NTFS driver)
   Justification:
 Without this patch, users using a USB stick formatted with this
 filesystem will see 'file exists' errors on some working copy
 operations. (This patch should have been part of the r896915
 group). I certainly don't recommend using FAT32 to host a
 subversion working copy. But AnkhSVN reports show that it is a
 common user scenario.
   Votes:
 +1: rhuijben, pburba

This patch needs one more vote. Anybody?

Without this patch I turn my +1 on r896915 to a -1.

We rolled this patch in an AnkhSVN test version to investigate issue #3574 and 
we got a lot of errors caused by just this patch reported.

I don't want to cause the same error for more Subversion users.

Bert

RE: svn commit: r929382 - /subversion/branches/1.6.x-wc-ng-error/subversion/libsvn_wc/questions.c

[Bert Huijben]

> -Original Message-
> From: Peter Samuelson [mailto:pe...@p12n.org]
> Sent: woensdag 31 maart 2010 18:33
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r929382 - /subversion/branches/1.6.x-wc-ng-
> error/subversion/libsvn_wc/questions.c
> 
> 
> [Daniel Shahaf]
> > > @@ -62,7 +63,12 @@ is_inside_wc_ng(const char *abspath,
> > >
> > >wc_db_path = svn_path_join_many(pool, abspath,
> SVN_WC_ADM_DIR_NAME,
> > >"wc.db", NULL);
> >
> > Shouldn't this use svn_wc_get_adm_dir()?
> 
> Probably ... but since this is 1.6-specific code (it is not and never
> will be in trunk), I think we'd need a good reason to bother changing
> it.

Is the common use of "_svn" by setting an environment variable, on Windows a
good enough reason for you?

Bert

RE: IRC build failure notifications

[Bert Huijben]

> -Original Message-
> From: Daniel Shahaf [mailto:d...@daniel.shahaf.name]
> Sent: donderdag 1 april 2010 21:49
> To: dev@subversion.apache.org
> Subject: IRC build failure notifications
> 
> The ASF-hosted buildbot can report on IRC (to a channel or in privmsg)
> the
> status of builds.  There was disagreement today on #svn-dev whether
> it's
> desired.  What do people think of having the bot notify on IRC
> 
> (a) after every failed build?
> (b) on the first failed build after a series of successful builds?
> (c) on the first successful build after a series of failed builds?

+0 on B and C. (I made the bot send me a privmsg on more than that)

Yes, I like that we keep an eye on the bot, but I don't think we should
flood #svn-dev with all kinds of log messages.

Bert

FW: svn commit: r930702 - /subversion/trunk/subversion/tests/libsvn_client/client-test.c

[Bert Huijben]

> -Original Message-
> From: rhuij...@apache.org [mailto:rhuij...@apache.org]
> Sent: zondag 4 april 2010 17:31
> To: comm...@subversion.apache.org
> Subject: svn commit: r930702 -
> /subversion/trunk/subversion/tests/libsvn_client/client-test.c
> 
> Author: rhuijben
> Date: Sun Apr  4 15:31:09 2010
> New Revision: 930702
> 
> URL: http://svn.apache.org/viewvc?rev=930702&view=rev
> Log:
> Make the patch test in client-test pass on Windows by removing the
> assumption that the to be patched file has platform dependent EOLs. It
> has "\n" as end of line marker on all platforms.

Stefan or Daniel,

Can one of you (or maybe somebody else) confirm that this is the designed 
behavior?

Thanks,

Bert
> 
> * subversion/tests/libsvn_client/client-test.c
>   (check_patch_result): Add eol argument and pass this value to
> svn_stream_readline.
>   (test_patch): Assume "\n" line endings on the tempfile, but native
> eols on the reject file.
> 
> Modified:
> subversion/trunk/subversion/tests/libsvn_client/client-test.c
> 
> Modified: subversion/trunk/subversion/tests/libsvn_client/client-test.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_c
> lient/client-test.c?rev=930702&r1=930701&r2=930702&view=diff
> ===
> ===
> --- subversion/trunk/subversion/tests/libsvn_client/client-test.c
> (original)
> +++ subversion/trunk/subversion/tests/libsvn_client/client-test.c Sun
> Apr  4 15:31:09 2010
> @@ -206,10 +206,10 @@ test_args_to_target_array(apr_pool_t *po
> 
> 
>  /* A helper function for test_patch().
> - * It compares a patched or reject file against expected content.
> - * It also deletes the file if the check was successful. */
> + * It compares a patched or reject file against expected content using
> the
> + *  specified EOL. It also deletes the file if the check was
> successful. */
>  static svn_error_t *
> -check_patch_result(const char *path, const char **expected_lines,
> +check_patch_result(const char *path, const char **expected_lines,
> const char *eol,
> int num_expected_lines, apr_pool_t *pool)
>  {
>svn_stream_t *stream;
> @@ -226,7 +226,7 @@ check_patch_result(const char *path, con
> 
>svn_pool_clear(iterpool);
> 
> -  SVN_ERR(svn_stream_readline(stream, &line, APR_EOL_STR, &eof,
> pool));
> +  SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
>if (i < num_expected_lines)
>  if (strcmp(expected_lines[i++], line->data) != 0)
>return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
> @@ -357,14 +357,14 @@ test_patch(const svn_test_opts_t *opts,
>key = "A/D/gamma";
>patched_tempfile_path = apr_hash_get(patched_tempfiles, key,
> APR_HASH_KEY_STRING);
> -  SVN_ERR(check_patch_result(patched_tempfile_path, expected_gamma,
> +  SVN_ERR(check_patch_result(patched_tempfile_path, expected_gamma,
> "\n",
>   EXPECTED_GAMMA_LINES, pool));
>SVN_ERR_ASSERT(apr_hash_count(reject_tempfiles) == 1);
>key = "A/D/gamma";
>reject_tempfile_path = apr_hash_get(reject_tempfiles, key,
>   APR_HASH_KEY_STRING);
>SVN_ERR(check_patch_result(reject_tempfile_path,
> expected_gamma_reject,
> - EXPECTED_GAMMA_REJECT_LINES, pool));
> + APR_EOL_STR, EXPECTED_GAMMA_REJECT_LINES,
> pool));
> 
>return SVN_NO_ERROR;
>  }
> 

RE: r877014 - on Windows, invalid path => svn_node_none [was: svn commit: r930333 - /subversion/branches/1.6.x/STATUS]

[Bert Huijben]

> -Original Message-
> From: Paul Burba [mailto:ptbu...@gmail.com]
> Sent: dinsdag 6 april 2010 21:59
> To: Julian Foad
> Cc: rhuij...@apache.org; dev@subversion.apache.org
> Subject: Re: r877014 - on Windows, invalid path => svn_node_none [was: svn
> commit: r930333 - /subversion/branches/1.6.x/STATUS]
> 
> On Tue, Apr 6, 2010 at 10:53 AM, Paul Burba  wrote:
> > On Tue, Apr 6, 2010 at 10:22 AM, Paul Burba  wrote:
> >> On Tue, Apr 6, 2010 at 8:39 AM, Julian Foad 
> wrote:
> >>> Hi Paul, Bert.
> >>>
> >>> I'm wondering about this r877014 change, which is proposed for 1.6.x
> >>> back-port:
> >>>
> >>> [[[
> >>> r877014 | rhuijben | 2009-04-02 14:01:48 +0100 (Thu, 02 Apr 2009) | 7
> lines
> >>>
> >>> * subversion/libsvn_subr/io.c
> >>>  (io_check_path): On Windows, treat a path containing invalid characters
> as
> >>> a non-existing path. (We already detected invalid drives and invalid
> >>> network paths.) This enables locating the repository root via
> >>> svn_repos_find_root_path() in cases like:
> >>> $ svn mkdir --parents file:///G:/repos/d/f:r/s:t -m ""
> >>>
> >>> 
> >>>   if (APR_STATUS_IS_ENOENT(apr_err))
> >>> *kind = svn_node_none;
> >>> -  else if (APR_STATUS_IS_ENOTDIR(apr_err))
> >>> +  else if (APR_STATUS_IS_ENOTDIR(apr_err)
> >>> +#if WIN32
> >>> +   || (APR_TO_OS_ERROR(apr_err) == ERROR_INVALID_NAME)
> >>> +#endif
> >>> +   )
> >>> *kind = svn_node_none;
> >>>   else if (apr_err)
> >>> return svn_error_wrap_apr(apr_err, _("Can't check path '%s'"),
> >>> ]]]
> >>>
> >>> There seems to be a funny interdependency between io_check_path()
> and
> >>> check_repos_path() and svn_repos_find_root_path().
> >>>
> >>> io_check_path() returns 'none' if the requested entry is not on disk,
> >>> obviously, but this change now makes it also return 'none' if the name
> >>> is invalid, which it didn't do before.
> >>>
> >>> check_repos_path() says "Return TRUE on errors (which would be
> >>> permission errors, probably)", which is a rather rash assumption.
> >>
> >> Hi Julian,
> >>
> >> Never mind the rash assumption, how about the fact that this function
> >> is effectively asked "is PATH the root of a repository, yes or no?"
> >> and answers "yes" when PATH is in fact is the root of a repository and
> >> "yes*" when it is not.
> >>
> >> * "Yes it is, actually wait, we are just kidding it isn't, but don't
> >> worry, you'll find that out later!"
> >>
> >>> svn_repos_find_root_path() first checks whether the path has some
> kinds
> >>> of invalid characters:
> >>>
> >>> [[[
> >>>  /* Try to decode the path, so we don't fail if it contains characters
> >>> that aren't supported by the OS filesystem.  The subversion fs
> >>> isn't restricted by the OS filesystem character set. */
> >>>  err = svn_utf_cstring_from_utf8(&decoded, candidate, pool);
> >>>  if (!err && check_repos_path(candidate, pool))
> >>>[then return 'candidate']
> >>> ]]]
> >>>
> >>> It looks like the desired effect is being achieved by a rather oddly
> >>> layered set of assumptions and interactions in this chain of functions.
> >>
> >> Yeah, odd indeed.  r877014's change to io_check_path() is really about
> >> making svn_repos_find_root_path() not choke on check_repos_path()'s
> >> bizzare semantic of returning TRUE when a path is not in fact the root
> >> of a repository.  Perhaps we should fix svn_repos_find_root_path()
> >> directly?
> >
> > Bert,
> >
> > Wouldn't reverting r877014 and moving the fix into
> > repos.c:check_repos_path() like so...
> >
> > [[[
> > Index: subversion/libsvn_repos/repos.c
> >
> ==
> =
> > --- subversion/libsvn_repos/repos.c (revision 931168)
> > +++ subversion/libsvn_repos/repos.c (working copy)
> > @@ -1417,6 +1417,13 @@
> >   &kind, pool);
> >   if (err)
> > {
> > +#ifdef WIN32
> > +  if (APR_TO_OS_ERROR(err->apr_err) == ERROR_INVALID_NAME)
> > +{
> > +  svn_error_clear(err);
> > +  return FALSE;
> > +}
> > +#endif
> >   svn_error_clear(err);
> >   return TRUE;
> > }
> > ]]]
> >
> > ...solve the problem r877014 was intended to address without changing
> > the semantics of svn_io_check_path()?
> >
> > (trying this right now)
> >
> > Paul
> >
> >>> Changing [svn_]io_check_path() has far wider potential repercussions
> >>> than just the intended result.
> >>>
> >>> What do you think?
> >>
> >> I've changed my vote to -0.  I can't point to any specific problems,
> >> but upon further reflection, I don't feel entirely comfortable saying
> >> this change won't cause other problems.
> >>
> >> Hoping Bert can weigh in on this...
> 
> Hi Bert,
> 
> A few questions below...
> 
> From IRC:
> 
> >  julianf, pburba: I see no issue with moving the check in the
> >repos code. But I think that we should avoid ch

RE: svn commit: r931209 - /subversion/trunk/subversion/libsvn_ra/ra_loader.c

[Bert Huijben]

> -Original Message-
> From: cmpil...@apache.org [mailto:cmpil...@apache.org]
> Sent: dinsdag 6 april 2010 18:25
> To: comm...@subversion.apache.org
> Subject: svn commit: r931209 -
> /subversion/trunk/subversion/libsvn_ra/ra_loader.c
> 
> Author: cmpilato
> Date: Tue Apr  6 16:25:21 2010
> New Revision: 931209
> 
> URL: http://svn.apache.org/viewvc?rev=931209&view=rev
> Log:
> Add compat code to work around pre-1.7 servers' messy handling of
> mergeinfo paths.
> 
> * subversion/libsvn_ra/ra_loader.c
>   (svn_ra_get_mergeinfo): Strip leading slashes from catalog keys
> returned by the RA provider.

Shouldn't we handle this in the specific RA layers instead of globally for all 
ra-layers?

That would allow removing the code for future code paths (like we did for 
HTTPv2), while this code would hide the issues if we ever reintroduce the bug?

I'm pretty sure you fixed ra_local, so that is at least one ra layer that 
doesn't need this fix.

Bert 

RE: r877014 - on Windows, invalid path => svn_node_none [was: svn commit: r930333 - /subversion/branches/1.6.x/STATUS]

[Bert Huijben]

> -Original Message-
> From: Bert Huijben [mailto:b...@qqmail.nl]
> Sent: woensdag 7 april 2010 11:09
> To: 'Paul Burba'; 'Julian Foad'
> Cc: rhuij...@apache.org; dev@subversion.apache.org
> Subject: RE: r877014 - on Windows, invalid path => svn_node_none [was: svn
> commit: r930333 - /subversion/branches/1.6.x/STATUS]
> 
> 
> 
> > -Original Message-
> > From: Paul Burba [mailto:ptbu...@gmail.com]
> > Sent: dinsdag 6 april 2010 21:59
> > To: Julian Foad
> > Cc: rhuij...@apache.org; dev@subversion.apache.org
> > Subject: Re: r877014 - on Windows, invalid path => svn_node_none [was:
> svn
> > commit: r930333 - /subversion/branches/1.6.x/STATUS]
> >
> > On Tue, Apr 6, 2010 at 10:53 AM, Paul Burba  wrote:
> > > On Tue, Apr 6, 2010 at 10:22 AM, Paul Burba 
> wrote:
> > >> On Tue, Apr 6, 2010 at 8:39 AM, Julian Foad
> 
> > wrote:
> > >>> Hi Paul, Bert.
> > >>>
> > >>> I'm wondering about this r877014 change, which is proposed for 1.6.x
> > >>> back-port:
> > >>>
> > >>> [[[
> > >>> r877014 | rhuijben | 2009-04-02 14:01:48 +0100 (Thu, 02 Apr 2009) | 7
> > lines
> > >>>
> > >>> * subversion/libsvn_subr/io.c
> > >>>  (io_check_path): On Windows, treat a path containing invalid
> characters
> > as
> > >>> a non-existing path. (We already detected invalid drives and invalid
> > >>> network paths.) This enables locating the repository root via
> > >>> svn_repos_find_root_path() in cases like:
> > >>> $ svn mkdir --parents file:///G:/repos/d/f:r/s:t -m ""
> > >>>
> > >>> 
> > >>>   if (APR_STATUS_IS_ENOENT(apr_err))
> > >>> *kind = svn_node_none;
> > >>> -  else if (APR_STATUS_IS_ENOTDIR(apr_err))
> > >>> +  else if (APR_STATUS_IS_ENOTDIR(apr_err)
> > >>> +#if WIN32
> > >>> +   || (APR_TO_OS_ERROR(apr_err) == ERROR_INVALID_NAME)
> > >>> +#endif
> > >>> +   )
> > >>> *kind = svn_node_none;
> > >>>   else if (apr_err)
> > >>> return svn_error_wrap_apr(apr_err, _("Can't check path '%s'"),
> > >>> ]]]
> > >>>
> > >>> There seems to be a funny interdependency between
> io_check_path()
> > and
> > >>> check_repos_path() and svn_repos_find_root_path().
> > >>>
> > >>> io_check_path() returns 'none' if the requested entry is not on disk,
> > >>> obviously, but this change now makes it also return 'none' if the name
> > >>> is invalid, which it didn't do before.
> > >>>
> > >>> check_repos_path() says "Return TRUE on errors (which would be
> > >>> permission errors, probably)", which is a rather rash assumption.
> > >>
> > >> Hi Julian,
> > >>
> > >> Never mind the rash assumption, how about the fact that this function
> > >> is effectively asked "is PATH the root of a repository, yes or no?"
> > >> and answers "yes" when PATH is in fact is the root of a repository and
> > >> "yes*" when it is not.
> > >>
> > >> * "Yes it is, actually wait, we are just kidding it isn't, but don't
> > >> worry, you'll find that out later!"
> > >>
> > >>> svn_repos_find_root_path() first checks whether the path has some
> > kinds
> > >>> of invalid characters:
> > >>>
> > >>> [[[
> > >>>  /* Try to decode the path, so we don't fail if it contains 
> > >>> characters
> > >>> that aren't supported by the OS filesystem.  The subversion fs
> > >>> isn't restricted by the OS filesystem character set. */
> > >>>  err = svn_utf_cstring_from_utf8(&decoded, candidate, pool);
> > >>>  if (!err && check_repos_path(candidate, pool))
> > >>>[then return 'candidate']
> > >>> ]]]
> > >>>
> > >>> It looks like the desired effect is being achieved by a rather oddly
> > >>> layered set of assumptions and interactions in this chain of functions.
> &

RE: r877014 - on Windows, invalid path => svn_node_none [was: svn commit: r930333 - /subversion/branches/1.6.x/STATUS]

[Bert Huijben]

> -Original Message-
> From: Paul Burba [mailto:ptbu...@gmail.com]
> Sent: woensdag 7 april 2010 15:27
> To: Bert Huijben
> Cc: Julian Foad; rhuij...@apache.org; dev@subversion.apache.org
> Subject: Re: r877014 - on Windows, invalid path => svn_node_none [was: svn
> commit: r930333 - /subversion/branches/1.6.x/STATUS]
> 
> On Wed, Apr 7, 2010 at 5:09 AM, Bert Huijben  wrote:
> >
> >
> >> -Original Message-
> >> From: Paul Burba [mailto:ptbu...@gmail.com]
> >> Sent: dinsdag 6 april 2010 21:59
> >> To: Julian Foad
> >> Cc: rhuij...@apache.org; dev@subversion.apache.org
> >> Subject: Re: r877014 - on Windows, invalid path => svn_node_none [was:
> svn
> >> commit: r930333 - /subversion/branches/1.6.x/STATUS]
> >>
> >> On Tue, Apr 6, 2010 at 10:53 AM, Paul Burba 
> wrote:
> >> > On Tue, Apr 6, 2010 at 10:22 AM, Paul Burba 
> wrote:
> >> >> On Tue, Apr 6, 2010 at 8:39 AM, Julian Foad
> 
> >> wrote:
> >> >>> Hi Paul, Bert.
> >> >>>
> >> >>> I'm wondering about this r877014 change, which is proposed for 1.6.x
> >> >>> back-port:
> >> >>>
> >> >>> [[[
> >> >>> r877014 | rhuijben | 2009-04-02 14:01:48 +0100 (Thu, 02 Apr 2009) | 7
> >> lines
> >> >>>
> >> >>> * subversion/libsvn_subr/io.c
> >> >>>  (io_check_path): On Windows, treat a path containing invalid
> characters
> >> as
> >> >>> a non-existing path. (We already detected invalid drives and 
> >> >>> invalid
> >> >>> network paths.) This enables locating the repository root via
> >> >>> svn_repos_find_root_path() in cases like:
> >> >>> $ svn mkdir --parents file:///G:/repos/d/f:r/s:t -m ""
> >> >>>
> >> >>> 
> >> >>>   if (APR_STATUS_IS_ENOENT(apr_err))
> >> >>> *kind = svn_node_none;
> >> >>> -  else if (APR_STATUS_IS_ENOTDIR(apr_err))
> >> >>> +  else if (APR_STATUS_IS_ENOTDIR(apr_err)
> >> >>> +#if WIN32
> >> >>> +   || (APR_TO_OS_ERROR(apr_err) == ERROR_INVALID_NAME)
> >> >>> +#endif
> >> >>> +   )
> >> >>> *kind = svn_node_none;
> >> >>>   else if (apr_err)
> >> >>> return svn_error_wrap_apr(apr_err, _("Can't check path '%s'"),
> >> >>> ]]]
> >> >>>
> >> >>> There seems to be a funny interdependency between
> io_check_path()
> >> and
> >> >>> check_repos_path() and svn_repos_find_root_path().
> >> >>>
> >> >>> io_check_path() returns 'none' if the requested entry is not on disk,
> >> >>> obviously, but this change now makes it also return 'none' if the
> name
> >> >>> is invalid, which it didn't do before.
> >> >>>
> >> >>> check_repos_path() says "Return TRUE on errors (which would be
> >> >>> permission errors, probably)", which is a rather rash assumption.
> >> >>
> >> >> Hi Julian,
> >> >>
> >> >> Never mind the rash assumption, how about the fact that this function
> >> >> is effectively asked "is PATH the root of a repository, yes or no?"
> >> >> and answers "yes" when PATH is in fact is the root of a repository and
> >> >> "yes*" when it is not.
> >> >>
> >> >> * "Yes it is, actually wait, we are just kidding it isn't, but don't
> >> >> worry, you'll find that out later!"
> >> >>
> >> >>> svn_repos_find_root_path() first checks whether the path has some
> >> kinds
> >> >>> of invalid characters:
> >> >>>
> >> >>> [[[
> >> >>>  /* Try to decode the path, so we don't fail if it contains 
> >> >>> characters
> >> >>> that aren't supported by the OS filesystem.  The subversion fs
> >> >>> isn't restricted by the OS filesystem character set. */
> >> >>>  err = svn_utf_cstring_from_utf8(&decoded, candidate, pool);
> >&

RE: fourth tree: "INHERITED" (was: wc-ng base/working nodes in a copied tree)

[Bert Huijben]

> -Original Message-
> From: Greg Stein [mailto:gst...@gmail.com]
> Sent: woensdag 7 april 2010 23:25
> To: Philip Martin
> Cc: dev@subversion.apache.org
> Subject: Re: fourth tree: "INHERITED" (was: wc-ng base/working nodes in
> a copied tree)
> 
> After some further discussion on IRC, and some thought...
> 
> I think this may be more of a representational problem, and might not
> be a "true" fourth tree. Especially because supporting the revert
> scenario actually implies N trees. Bert tried to describe this a while
> back, but I didn't understand his description (too many "A" nodes).
> Consider the following:
> 
> $ svn cp A X  # copies A/Y/Z/file
> $ svn cp B X/Y  # copies B/Z/file
> $ svn cp C X/Y/Z  # copies C/file
> $ svn cp file X/Y/Z/file
> 
> We have four operation roots, and four layers of "file". Reverting
> each op-root will reveal the previous layer.
> 
> In 1.6, we probably had just one layer, but if we're going to solve
> this, then let's do it right.
> 
> I propose that we create a new table called NODE_DATA which is keyed
> by . The first two are the usual, and
> op_depth is the "operation depth". In the above example, we have four
> WORKING_NODE rows, each establishing an operation root, with
> local_relpath values of [X, X/Y, X/Y/Z, X/Y/Z/file]. In the NODE_DATA
> table, we have the following four rows:
> 
> <1, X/Y/Z/file, 1>  # from the X op-root
> <1, X/Y/Z/file, 2>  # from the X/Y op-root
> <1, X/Y/Z/file, 3>  # from the X/Y/Z op-root
> <1, X/Y/Z/file, 4>  # from the X/Y/Z/file op-root
> 
> Essentially, op_depth = oproot_relpath.count('/') + 1
> 
> We can record BASE node data as op_depth == 0.
> 
> Looking up the data for "file" is a query like this:
> 
> SELECT * from NODE_DATA
> WHERE wc_id = ?1 AND local_relpath = ?2
> ORDER BY op_depth DESC
> LIMIT 1;
> 
> That provides the "current" file data.
> 
> Some of the common columns between BASE_NODE and WORKING_NODE move to
> this new NODE_DATA table. I think they are:
> 
>   kind, [checksum], changed_*, properties

I think you need some kind of 'not-present'/'excluded' status in this
copied/inherited/fourth tree too.

If you have a working copy

/A
/A/B
/A/B/C

Where you exclude C

/A
/A/B
[/A/B/C] (excluded)

Then you make a copy of /A to /D

/A
[/A/B/C] (excluded)
/D (copy)
/D/B (child of copy)
[/D/B/C] (excluded child of copy)

And then we delete /D/B

/A
/A/B[/C]
/D (copy)
/D/B (deleted)

Then I want to revert the delete of /D/B.

/A
/A/B[/C]
/D (copy)
/D/B (child of copy)

Note that there is no:
 [/D/B/C] (excluded child of copy)

The new COPIED table allows me to get all the information on B back, but I
miss the information that it once had a child C that was excluded.

So if I commit the tree I get a working copy that assumes that I still have
/D/B/C, because nothing recorded that I don't have that node locally.

Bert

RE: svn commit: r931162 - in /subversion/trunk: ./ subversion/libsvn_diff/parse-diff.c subversion/tests/cmdline/patch_tests.py

[Bert Huijben]

> -Original Message-
> From: s...@apache.org [mailto:s...@apache.org]
> Sent: dinsdag 6 april 2010 16:20
> To: comm...@subversion.apache.org
> Subject: svn commit: r931162 - in /subversion/trunk: ./
> subversion/libsvn_diff/parse-diff.c subversion/tests/cmdline/patch_tests.py
> 
> Author: stsp
> Date: Tue Apr  6 14:20:21 2010
> New Revision: 931162
> 
> URL: http://svn.apache.org/viewvc?rev=931162&view=rev
> Log:
> Cherry-pick r929288 and r931140 from the svn-patch-improvements branch
> to trunk, fixing the problem reported in:
> 
>   Date: Wed, 3 Mar 2010 15:52:59 +0100
>   From: Stefan Sperling
>   To: dev@subversion.apache.org
>   Subject: svn patch fails on this diff
>   Message-ID: <20100303145259.gf8...@noel.stsp.name>
>   http://svn.haxx.se/dev/archive-2010-03/0097.shtml
>   http://svn.haxx.se/dev/archive-2010-03/0098.shtml (attachment)
> 


 
> Modified: subversion/trunk/subversion/tests/cmdline/patch_tests.py
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/
> patch_tests.py?rev=931162&r1=931161&r2=931162&view=diff
> ==
> 
> --- subversion/trunk/subversion/tests/cmdline/patch_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/patch_tests.py Tue Apr  6
> 14:20:21 2010
> @@ -841,6 +841,92 @@ def patch_strip1(sbox):
> 1, # dry-run
> '-p1')
> 
> +def patch_no_index_line(sbox):
> +  "patch with no index lines"
> +
> +  sbox.build()
> +  wc_dir = sbox.wc_dir
> +
> +  patch_file_path =
> tempfile.mkstemp(dir=os.path.abspath(svntest.main.temp_dir))[1]

This line reintroduces the issue I fixed in the other tests in r930990.

Stefan: Answering your question: It seems the handle must be closed before 
leaving the function. (I don't mind where, as long as it fixes the errors)

Greg: You just fixed several other functions to use open(...).read(), without 
closing the handle. This doesn't cause the same error, so this file is somehow 
closed without the explicit close required above. Do you have any idea why 
mkstemp() does need the explicit close? (Python bug?)


Other question: Do we really need a mkstemp call here? I assumed we have a per 
test temporary directory where we can place any named file we want?

Bert

FW: svn commit: r931738 - in /subversion/trunk/subversion/tests/cmdline: diff_tests.py merge_tests.py

[Bert Huijben]

> -Original Message-
> From: gst...@apache.org [mailto:gst...@apache.org]
> Sent: donderdag 8 april 2010 2:28
> To: comm...@subversion.apache.org
> Subject: svn commit: r931738 - in
> /subversion/trunk/subversion/tests/cmdline: diff_tests.py merge_tests.py
> 
> Author: gstein
> Date: Thu Apr  8 00:28:17 2010
> New Revision: 931738
> 
> URL: http://svn.apache.org/viewvc?rev=931738&view=rev
> Log:
> XFail two more tests that throw errors because we cannot represent a
> local-add within a copied subtree.
> 
> * subversion/tests/cmdline/diff_tests.py:
>   (diff_in_renamed_folder): add comment about breakage
>   (test_list): XFail the above test
> 
> * subversion/tests/cmdline/merge_tests.py:
>   (merge_catches_nonexistent_target): add comment about breakage
>   (test_list): XFail the above test

svnsync_tests.py 16 needs the same handling. (It also fails on a local add 
below a copy).
(This test is skipped on ra_local)

Bert 

RE: Severe performance issues with large directories

[Bert Huijben]

> -Original Message-
> From: Paul Holden [mailto:paul.hol...@gmail.com]
> Sent: vrijdag 9 april 2010 13:32
> To: dev@subversion.apache.org
> Subject: Severe performance issues with large directories
> 
> Hello,
> 
> 
> 
> I've had a look through the issue tracker and mailing list archives and
> didn't find any references to this issue. I also assume that this is a
more
> appropriate mailing list than 'users'.

I think you can find a lot of issues similar to your issue in our issue
tracker.

For Subversion 1.7 we are rewriting the entire working copy library to use a
database which should resolve most of your issues. (And which will allow us
to resolve more issues in future versions). The issues related to this
rewrite have the 'WC-NG' name somewhere.

> We've noticed recently that we have terrible performance when updating a
> particular directory in our repository. We've realised that the poor
> performance is related to the fact that we have 5,800 or so files in a
> single directory. (I know! This is far from ideal but we're a long way
into
> development and reorganising the directory structure at this stage is very
> difficult.)
> 
That is certainly a number of files where the entries handling in the
current wc library will be slow. (Confirming your findings later in your
mail)
 
> To give some concrete numbers, we recently re-exported about 10,000
> texture
> files (averaging about 40KB each, or 390MB) and 5,800 shaders (averaging
> about 4KB each, or 22MB total). Both these files are gzip compressed. Here
> are some approximate times for 'svn up'
> 
> 
> 
> Textures: 10,000 files, 390MB, ~4 minutes
> 
> Shaders: 5,800 files, 22MB, ~10 minutes
> 
> 
> 
> The key point here is that the textures are nicely distributed in a
> well-organised directory structure, but the shaders are dumped into a
single
> directory.
> 
> 
> 
> The problem we face now is that we're iterating as lot on the engine,
which
> is causing us to rebuild the shaders every day.
> 
> 
> 
> To cut a long story short, I ran SysInternals procmon.exe while svn was
> updating, and saw two alarming behaviours:
> 
> 
> 
> 1) .svn\entries is being read in its entirety (in 4kb chunks) for *every*
> file that's updated in the directory. As the shaders dir contains so many
> files, it's approximately 1MB in size. That's 5,800 reads of a 1MB file
> (5.8GB in total) for a single update! I know this file is likely to be
> cached by the OS, but that's still a lot of unnecessary system calls and
> memory being copied around. Please excuse my ignorance if there's a
> compelling reason to re-read this file multiple times, but can't
subversion
> cache the contents of this file when it's updating the directory?
Presumably
> it's locked the directory at this point, so it can be confident that the
> contents of this file won't be changed externally?

For WC-NG we move all the entries data in a single wc.db file in a .svn
directory below the root of your working copy. This database is accessed via
SQLite, so it doesn't need the chunked rewriting or anything of that. (It
even has in-memory caching and transaction handling, so we don't have to do
that in Subversion itself any more)

> 2) subversion appears to generate a temporary file in .svn\prop-base\ for
> every file that's being updated. It's generating filenames sequentially,
> which means that when 5,800 files are being updated it ends up doing this:
> 
> 
> 
> file_open tempfile.tmp? Already exists!
> 
> file_open tempfile.2.tmp? Already exists!
> 
> file_open tempfile.3.tmp? Already exists!
> 
> ...some time later
> 
> file_open tempfile.5800.tmp? Yes!

Wow.

Are you sure that this is in prop-base, not .svn/tmp?

For 1.7 we made the tempfilename generator better in guessing new names, but
for property handling we won't be using files in 1.7. (Looking at these
numbers and those that follow later in your mail, we might have to look in
porting some of this back to 1.6).

Properties will be moved in wc.db, to remove the file accesses completely.
(We can update them with the node information in a single transaction;
without additional file accesses)

> For N files in a directory, that means subversion ends up doing (n^2 +
n)/2
> calls to file_open. In our case that means it's testing for file existence
> 16,822,900 times (!) in order to do a full update. Even with just 100
files
> in a directory that's 5,050 tests.
> 
> 
> 
> Is there any inherent reason these files need to be generated
sequentially?
> From reading the comments in 'svn_io_open_uniquely_named' it sounds like
> these files are named sequentially for the benefit of people looking at
> conflicts in their working directory. As these files are being generated
> within the 'magic' .svn folder, is there any reason to number them
> sequentially? Just calling rand() until there were no collisions would
> probably give a huge increase in performance.

In 1.7 we have a new api that uses a smarter algorithm, but we can't add
public apis to 1.6

RE: Properties (was Re: Subversion Vision and Roadmap Proposal)

[Bert Huijben]

> -Original Message-
> From: Greg Stein [mailto:gst...@gmail.com]
> Sent: donderdag 8 april 2010 22:41
> To: C. Michael Pilato
> Cc: dev@subversion.apache.org
> Subject: Re: Properties (was Re: Subversion Vision and Roadmap
> Proposal)
> 
> On Thu, Apr 8, 2010 at 15:35, C. Michael Pilato 
> wrote:
> > Martin Hauner wrote:
> >> Hi,
> >>
> >> On 06.04.10 00:01, Stefan Sperling wrote:
> >>> On Mon, Apr 05, 2010 at 07:28:57PM +0200, Martin Hauner wrote:
>  [..]
>  With svn:mergeinfo I have to update after each commit because its
> on
>  my root folder and always is out of date on the next commit.
> >>>
> >>> The out-of-dateness really comes from the mixed-revision working
> >>> copy concept, not from mergeinfo.
> >>>
> >>> The root of your working copy is not out of date right after
> >>> the commit. It is at the HEAD revision after commit:
> >>>
> >>> $ ls
> >>> alpha    beta     epsilon/ gamma/
> >>> $ svn merge ^/trunk
> >>> --- Merging r2 through r4 into '.':
> >>> U    alpha
> >>> --- Recording mergeinfo for merge of r2 through r4 into '.':
> >>>   U   .
> >>> $ svn ci -m merge
> >>> Sending        .
> >>> Sending        alpha
> >>> Transmitting file data .
> >>> Committed revision 5.
> >>> $ svn info . | grep ^Rev
> >>> Revision: 5
> >>>
> >>> Maybe you do not commit the mergeinfo which is set on the root of
> your
> >>> working copy? If so, why not?
> >>
> >> I always commit merges on the root.
> >>
> >> I have tried to create an example, but it works in my test
> repository. I
> >> can't reproduce my "at work" behavior.
> >>
> >> It still complains that '.' is out of date most os the at work. And
> I'm
> >> 100% sure that it started when subversion went merge-tracking.
> >
> > Subversion has always disallowed commits of propchanges on a
> directory
> > that's not fully up to date.  Why?  For the simple reason that, after
> the
> > commit happens, that directory cannot be considered "at the new
> revision X"
> > because we never got the changes we were missing from the server.
> >
> > What happened with merge tracking is that propchanges on directories
> > suddenly became quite a bit more common.  Like ... after every merge.
> 
> If there are no propchanges on the directory in the interim, then why
> not allow the commit?
> 
> IOW, did we disallow because we were lazy, or is there a fundamental
> problem with allowing the commit to proceed?

We can only do this if there are no property changes on the directory *and*
when there are no children added or removed from the directory.

(This is not necessary an issue on committing, but when  we upgrade the
revision number of the directory, without updating the list of children this
list of children is out of date in a way that can't be fixed by updating).

Bert

RE: where should I start

[Bert Huijben]

> -Original Message-
> From: Saiho Yuen [mailto:adenin...@yahoo.com]
> Sent: zondag 11 april 2010 19:10
> To: dev@subversion.apache.org
> Subject: where should I start
> 
> Hi, I wish to build a custom SVN GUI with Java. I have already
> downloaded the latest code source, but I have no idea where should I
> start. Can someone tell me where I can find the documentation for
> compiling the SVN project on Vista (which compiler to use?) and how to
> link the C library with Java (JNI).

Questions on using Subversion should be directed to the user list
(users{_AT_}subversion.apache.org). This list is about the development of
Subversion itself. So if you find a (serious) bug developing your client,
you are welcome to report it here.

As a first step to answering your question: Did you look at JavaHL? This is
a standard wrapping of our client api to java, and there are binaries
available to use it directly without compiling everything yourself. You need
the javahl jar/class files and a subversion distribution that installs
libsvnjavahl-1.dll. (I think the CollabNet setup has that binary, and the
SlikSvn binaries have it for x86 and x64).

Compiling Subversion itself is pretty easy, but it requires several
dependencies to be compiled before you can compile Subversion. And those
dependencies are the hard part of compiling.

I maintain a set of Scripts to compile Subversion on Windows as part of the
SharpSvn project. (Scripts are in
http://sharpsvn.open.collab.net/svn/sharpsvn/trunk/imports). With Visual
Studio 2005/2008, NAnt, ActiveState Python and ActiveState Perl installed
compiling should be as easy as running 'nant build' in that directory.

Bert 

RE: misaligned blame output if repo has >1m revisions

[Bert Huijben]

> -Original Message-
> From: Philipp Marek [mailto:philipp.ma...@emerion.com]
> Sent: maandag 12 april 2010 14:43
> To: Hyrum K. Wright
> Cc: dev; Erik Huelsmann; Stefan Sperling; Johan Corveleyn
> Subject: Re: misaligned blame output if repo has >1m revisions
> 
> Hello Hyrum!
> 
> On Montag, 12. April 2010, Hyrum K. Wright wrote:
> > On Mon, Apr 12, 2010 at 11:01 AM, Philipp Marek
> > > So 9 digits should buy a bit of time ;-)
> > Sure, but the more digits you put in, the more stuff gets pushed off the
> > back of the line.  On a 80-character terminal, with code that's 79
> > characters long, parsing blame output is difficult enough as it is;
adding
> > more space just seems like it would make the problem harder, and is only
> > required in a handful of cases.
> Maybe.
> 
> But
> a) even the linux kernel hackers aren't really strict about 80 columns;
> b) if you have more than 40 whitespace characters at the beginning of a
line,
>you can either set your tab-stops smaller, or at least scroll
horizontally
>to look for the line
> c) cutting the revision number or, as you say, the user, only creates a
bigger
>problem.

Well, on Windows consoles are all 80 characters wide. (You can fix this if
you are a frequent Command Prompt User, but most applications just assume 80
characters on Windows. And in many cases Windows switches back to 80
characters if it detects direct screen operations)

The tab width is not globally configurable on Windows.

> > And we still have the problem of too-long user names (such as
jerenkrantz
> >  in our own repo).
> 
> Well, my use-case is normally "why did this line change?" - and on a blame
> output I can easily go to the same line, too see the revision.
> So the width of the terminal is normally not interesting for me here.
> 
> Depending on your editor you could define a vertical fold, or have the
> revision/user in a status line or mouse popup, or whatever - then the
> additional space is of no concern. Only the full data, not cut off, must
be
> available.
> 
> 
> Of course, if the needed number of characters for revision and user is
simply
> found during traversal, so much the better.

We have all these values in memory during traversal so calculating it
shouldn't be a performance issue.

Bert

RE: svn commit: r933938 - in /subversion/trunk: build.conf subversion/include/private/svn_sqlite.h subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/sqlite.c subversion/libsvn_subr/sqlite.sql

[Bert Huijben]

> -Original Message-
> From: phi...@apache.org [mailto:phi...@apache.org]
> Sent: woensdag 14 april 2010 15:06
> To: comm...@subversion.apache.org
> Subject: svn commit: r933938 - in /subversion/trunk: build.conf
> subversion/include/private/svn_sqlite.h subversion/libsvn_fs_fs/fs_fs.c
> subversion/libsvn_subr/sqlite.c subversion/libsvn_subr/sqlite.sql
> 
> Author: philip
> Date: Wed Apr 14 13:05:56 2010
> New Revision: 933938
> 
> URL: http://svn.apache.org/viewvc?rev=933938&view=rev
> Log:
> Partial fix for issue 3596 (hotcopy and SQLite databases).
> 
> * subversion/include/private/svn_sqlite.c (svn_sqlite__hotcopy): New.
> 
> * subversion/libsvn_subr/sqlite.c (svn_sqlite__hotcopy): New.
> 
> * subversion/libsvn_subr/sqlite.sql: New file.
> 
> * build.conf (subr_sqlite): New.
> 
> * subversion/libsvn_fs_fs/fs_fs.c
>   (svn_fs_fs__hotcopy): Hotcopy rep cache, do it before the rev files.
> 
> Added:
> subversion/trunk/subversion/libsvn_subr/sqlite.sql
> Modified:
> subversion/trunk/build.conf
> subversion/trunk/subversion/include/private/svn_sqlite.h
> subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> subversion/trunk/subversion/libsvn_subr/sqlite.c
> 
> Modified: subversion/trunk/build.conf
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/build.conf?rev=933938&r1=
> 933937&r2=933938&view=diff
> ==
> 
> --- subversion/trunk/build.conf (original)
> +++ subversion/trunk/build.conf Wed Apr 14 13:05:56 2010
> @@ -380,6 +380,12 @@ type = sql-header
>  path = subversion/libsvn_wc
>  sources = wc-checks.sql
> 
> +[subr_sqlite]
> +description = Subversion SQLite interface
> +type = sql-header
> +path = subversion/libsvn_subr
> +sources = sqlite.sql
> +
> 
>  # 
> 
>  #
> 
> Modified: subversion/trunk/subversion/include/private/svn_sqlite.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private
> /svn_sqlite.h?rev=933938&r1=933937&r2=933938&view=diff
> ==
> 
> --- subversion/trunk/subversion/include/private/svn_sqlite.h (original)
> +++ subversion/trunk/subversion/include/private/svn_sqlite.h Wed Apr 14
> 13:05:56 2010
> @@ -310,6 +310,12 @@ svn_sqlite__with_transaction(svn_sqlite_
>   void *cb_baton, apr_pool_t *scratch_pool);
> 
> 
> +/* Hotcopy an SQLite database from SRC_PATH to DST_PATH. */
> +svn_error_t *
> +svn_sqlite__hotcopy(const char *src_path,
> +const char *dst_path,
> +apr_pool_t *scratch_pool);
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> 
> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs
> _fs.c?rev=933938&r1=933937&r2=933938&view=diff
> ==
> 
> --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Apr 14 13:05:56
> 2010
> @@ -1498,6 +1498,14 @@ svn_fs_fs__hotcopy(const char *src_path,
>/* Copy the config. */
>SVN_ERR(svn_io_dir_file_copy(src_path, dst_path, PATH_CONFIG, pool));
> 
> +  /* Copy the rep cache before copying the rev files to make sure all
> + cached references will be present in the copy. */
> +  src_subdir = svn_dirent_join(src_path, REP_CACHE_DB_NAME, pool);
> +  dst_subdir = svn_dirent_join(dst_path, REP_CACHE_DB_NAME, pool);
> +  SVN_ERR(svn_io_check_path(src_subdir, &kind, pool));
> +  if (kind == svn_node_file)
> +SVN_ERR(svn_sqlite__hotcopy(src_subdir, dst_subdir, pool));
> +
>/* Copy the min unpacked rev, and read its value. */
>if (format >= SVN_FS_FS__MIN_PACKED_FORMAT)
>  {
> @@ -1658,12 +1666,6 @@ svn_fs_fs__hotcopy(const char *src_path,
>  PATH_NODE_ORIGINS_DIR, TRUE, NULL,
>  NULL, pool));
> 
> -  /* Now copy the rep cache. */
> -  src_subdir = svn_dirent_join(src_path, REP_CACHE_DB_NAME, pool);
> -  SVN_ERR(svn_io_check_path(src_subdir, &kind, pool));
> -  if (kind == svn_node_file)
> -SVN_ERR(svn_io_dir_file_copy(src_path, dst_path,
> REP_CACHE_DB_NAME, pool));
> -
>/* Copy the txn-current file. */
>if (format >= SVN_FS_FS__MIN_TXN_CURRENT_FORMAT)
>  SVN_ERR(svn_io_dir_file_copy(src_path, dst_path, PATH_TXN_CURRENT,
> pool));
> 
> Modified: subversion/trunk/subversion/libsvn_subr/sqlite.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/sqli
> te.c?rev=933938&r1=933937&r2=933938&view=diff
> ==
> 
> --- subversion/trunk/subversion/libsvn_subr/sqlite.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/sqlite.c Wed Apr 14 13:05:56
> 201

RE: 1.6.11 tarballs up for testing/signing

[Bert Huijben]

> -Original Message-
> From: hy...@hyrumwright.org [mailto:hy...@hyrumwright.org] On Behalf
> Of Hyrum K. Wright
> Sent: donderdag 15 april 2010 19:19
> To: Subversion Development
> Subject: 1.6.11 tarballs up for testing/signing
> 
> 1.6.11 tarballs are up, the magic revision is r934486:
> 
> http://people.apache.org/~hwright/svn/1.6.11/

+1 for release

Tested [fsfs | bdb] x [ local | svn | serf | neon]
Windows 7 X64
Visual Studio 2008 SP1
(Compiling everything as X86 application)

APR 1.3.12
APR-Util 1.3.9
BDB 4.4.20
Cyrus Sasl 2.1.23
Neon 0.29.3
OpenSSL 1.0.0
Serf 0.3.1
SqLite 3.6.23
Swig 1.3.38

Apache httpd 2.2.14

(With IPv6 enabled in APR and Neon)

No test failures.

   Bert

-- subversion-1.6.11.tar.bz2.asc --
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (FreeBSD)

iEYEABECAAYFAkvIigcACgkQ/P1jBZgh97LrOgCgqBuNbOjq7IgrqrZDf7tKONyu
348AoMrl42yrscNV0aViRBFwHhj5uobH
=MhrW
-END PGP SIGNATURE-

-- subversion-1.6.11.tar.gz.asc --
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (FreeBSD)

iEYEABECAAYFAkvIigoACgkQ/P1jBZgh97JSnwCghGLfDJ50k1XPFD77tOmpHz1w
YtMAnRVV287i5Hu7qlAkMsdBwmnURQY/
=sDP8
-END PGP SIGNATURE-

-- subversion-1.6.11.zip.asc --
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (FreeBSD)

iEYEABECAAYFAkvIig0ACgkQ/P1jBZgh97K2ZACfWq+owzNBDKVPWGZmMOu4zQLY
ptoAn2EccKqCGbCD3sBfMZ/zKcDQW+GY
=wXI+
-END PGP SIGNATURE-

-- svn_version.h.dist.asc --
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (FreeBSD)

iEUEABECAAYFAkvIihAACgkQ/P1jBZgh97KBUACYgx/d9m3R9+xTTr0DlmWKOsRC
zwCeOkb6XGKLqH0df0FdQubyoymIVbE=
=fulk
-END PGP SIGNATURE-

RE: svn commit: r935413 - /subversion/trunk/subversion/libsvn_wc/adm_crawler.c

[Bert Huijben]

> -Original Message-
> From: gst...@apache.org [mailto:gst...@apache.org]
> Sent: zondag 18 april 2010 23:17
> To: comm...@subversion.apache.org
> Subject: svn commit: r935413 -
> /subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> 
> Author: gstein
> Date: Sun Apr 18 21:16:55 2010
> New Revision: 935413
> 
> URL: http://svn.apache.org/viewvc?rev=935413&view=rev
> Log:
> Fix potential URI encoding/decoding bug.
> 
> * subversion/libsvn_wc/adm_crawler.c:
>   (report_revisions_and_depths): wrap a couple comments. when the child
> is
> stripped off the URI, then decode it before comparison.
> 
> Modified:
> subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> 
> Modified: subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_
> crawler.c?rev=935413&r1=935412&r2=935413&view=diff
> ===
> ===
> --- subversion/trunk/subversion/libsvn_wc/adm_crawler.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_crawler.c Sun Apr 18
> 21:16:55 2010
> @@ -360,8 +360,8 @@ report_revisions_and_depths(svn_wc__db_t
>  }
>else
>  {
> -  /* We want to pull in the excluded target. So, report it
> as deleted,
> - and server will respond properly. */
> +  /* We want to pull in the excluded target. So, report it
> as
> + deleted, and server will respond properly. */
>if (! report_everything)
>  SVN_ERR(reporter->delete_path(report_baton,
>this_path, iterpool));
> @@ -424,10 +424,10 @@ report_revisions_and_depths(svn_wc__db_t
>else
>  missing = TRUE;
> 
> -  /* If a node is still missing from disk here, we have no way
> to recreate
> - it locally, so report as missing and move along.  Again,
> don't bother
> - if we're reporting everything, because the dir is already
> missing on
> - the server. */
> +  /* If a node is still missing from disk here, we have no way
> to
> + recreate it locally, so report as missing and move along.
> + Again, don't bother if we're reporting everything,
> because the
> + dir is already missing on the server. */
>if (missing && wrk_kind == svn_wc__db_kind_dir
> && (depth > svn_depth_files || depth ==
> svn_depth_unknown))
>  {
> @@ -450,7 +450,8 @@ report_revisions_and_depths(svn_wc__db_t
>const char *childname = svn_uri_is_child(dir_repos_relpath,
> this_repos_relpath,
> NULL);

s/svn_uri/svn_relpath/
> 
> -  if (!childname || strcmp(childname, child) != 0)
> +  if (childname == NULL
> +  || strcmp(svn_path_uri_decode(childname, iterpool),
> child) != 0)

A repos relpath is already uri_decoded, so this would break paths that have a 
literal %20 somewhere. (It would be correct if it was a real uri instead of a 
relpath)

Bert

RE: svn commit: r936163 - in /subversion/trunk/subversion/libsvn_wc: entries.c wc_db.c

[Bert Huijben]

> -Original Message-
> From: gst...@apache.org [mailto:gst...@apache.org]
> Sent: woensdag 21 april 2010 8:03
> To: comm...@subversion.apache.org
> Subject: svn commit: r936163 - in /subversion/trunk/subversion/libsvn_wc:
> entries.c wc_db.c
> 
> Author: gstein
> Date: Wed Apr 21 06:02:56 2010
> New Revision: 936163


> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_
> db.c?rev=936163&r1=936162&r2=936163&view=diff
> ==
> 
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Apr 21 06:02:56
> 2010
> @@ -5631,7 +5631,27 @@ svn_wc__db_scan_deletion(const char **ba
> 
>/* Only "normal" and "not-present" are allowed.  */
>SVN_ERR_ASSERT(base_presence == svn_wc__db_status_normal
> - || base_presence == svn_wc__db_status_not_present);
> + || base_presence == svn_wc__db_status_not_present
> +#if 1
> + /* ### there are cases where the BASE node is
> +### marked as incomplete. we should treat this
> +### as a "normal" node for the purposes of
> +### this function. we really should not allow
> +### it, but this situation occurs within the
> +### following tests:
> +###   switch_tests 31
> +###   update_tests 46
> +###   update_tests 53
> + */
> + || base_presence == svn_wc__db_status_incomplete
> +#endif
> + );

This syntax is non-standard and breaks compilation in Visual C (and probably 
other C preprocessors).

Bert

RE: svn commit: r936240 - /subversion/trunk/subversion/libsvn_wc/entries.c

[Bert Huijben]

> -Original Message-
> From: gst...@apache.org [mailto:gst...@apache.org]
> Sent: woensdag 21 april 2010 12:05
> To: comm...@subversion.apache.org
> Subject: svn commit: r936240 -
> /subversion/trunk/subversion/libsvn_wc/entries.c
> 
> Author: gstein
> Date: Wed Apr 21 10:04:42 2010
> New Revision: 936240
> 
> URL: http://svn.apache.org/viewvc?rev=936240&view=rev
> Log:
> Revamp and simplify the central entry-fetching function,
> svn_wc__get_entry. It no longer worries about the entries cache, and
> always goes to the database to fetch the parent/entry pair of entries.
> 
> * subversion/libsvn_wc/entries.c:
>   (svn_wc__get_entry): strip out the access baton and entries cacheing
> considerations from this function. use read_entry_pair to grab the
> target entry. only use the RESULT_POOL and SCRATCH_POOL rather than
> an
> access baton's pool.

This patch breaks 485 tests on the ubuntu buildbot and 479 on the Windows 
buildbot.

(I see basic_tests.py 1-17 in the list of failures)

Bert

RE: svn commit: r937033 - /subversion/trunk/subversion/libsvn_repos/dump.c

[Bert Huijben]

> -Original Message-
> From: pbu...@apache.org [mailto:pbu...@apache.org]
> Sent: donderdag 22 april 2010 21:40
> To: comm...@subversion.apache.org
> Subject: svn commit: r937033 -
> /subversion/trunk/subversion/libsvn_repos/dump.c
> 
> Author: pburba
> Date: Thu Apr 22 19:40:07 2010
> New Revision: 937033
> 
> URL: http://svn.apache.org/viewvc?rev=937033&view=rev
> Log:
> Issue an end-of-dump summary warning to supplement any in-line
> copy-source warnings so the latter don't get missed in a sea of output.
> 
> See http://svn.haxx.se/dev/archive-2010-04/0475.shtml
> 
> * subversion/libsvn_repos/dump.c
> 
>   (edit_baton): Add new member found_old_reference.
> 
>   (dump_node): Set new edit baton member if an old reference warning is
>issued.
> 
>   (svn_repos_dump_fs3): Possibly issue new summary warning.
> 
> Modified:
> subversion/trunk/subversion/libsvn_repos/dump.c
> 
> Modified: subversion/trunk/subversion/libsvn_repos/dump.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/d
> ump.c?rev=937033&r1=937032&r2=937033&view=diff
> ==
> 
> --- subversion/trunk/subversion/libsvn_repos/dump.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/dump.c Thu Apr 22 19:40:07
> 2010
> @@ -196,6 +196,10 @@ struct edit_baton
>/* The first revision dumped in this dumpstream. */
>svn_revnum_t oldest_dumped_rev;
> 
> +  /* Set to true if any references to revisions older than
> + OLDEST_DUMPED_REV were found in the dumpstream. */
> +  svn_boolean_t found_old_reference;
> +
>/* reusable buffer for writing file contents */
>char buffer[SVN__STREAM_CHUNK_SIZE];
>apr_size_t bufsize;
> @@ -428,7 +432,7 @@ dump_node(struct edit_baton *eb,
> " into an empty repository\n"
> "WARNING: will fail.\n"),
>   cmp_rev, eb->oldest_dumped_rev);
> -
> +  eb->found_old_reference = TRUE;
>SVN_ERR(eb->progress_func(eb->progress_baton,
>  eb->oldest_dumped_rev, warning, 
> pool));
>  }
> @@ -982,6 +986,7 @@ svn_repos_dump_fs3(svn_repos_t *repos,
>svn_revnum_t youngest;
>const char *uuid;
>int version;
> +  svn_boolean_t found_old_reference = FALSE;
> 
>/* Determine the current youngest revision of the filesystem. */
>SVN_ERR(svn_fs_youngest_rev(&youngest, fs, pool));
> @@ -1109,6 +1114,23 @@ svn_repos_dump_fs3(svn_repos_t *repos,
>  loop_end:
>if (progress_func)
>  SVN_ERR(progress_func(progress_baton, to_rev, NULL, subpool));
> +
> +  if (((struct edit_baton *)dump_edit_baton)->found_old_reference)
> +found_old_reference = TRUE;
> +}
> +
> +  /* Did we issue any warnings about references to revisions older than
> + the oldest dumped revision?  If so, then issue a final generic
> + warning, since the inline warnings already issued might easily be
> + missed. */
> +  if (found_old_reference)
> +{
> +  const char *warning = apr_psprintf(
> +subpool,
> +_("WARNING: The range of revisions dumped contained references
> to\n"
> +  "WARNING: copy sources outside that range.\n"));
> +  SVN_ERR(progress_func(progress_baton, SVN_INVALID_REVNUM,
> warning,
> +subpool));
>  }

This block needs a test for progress_func being NULL. (And doesn't need the 
apr_psprintf, but Greg already noticed that)

Bert

RE: Feature idea: user-configurable post-commit notifications

[Bert Huijben]

> -Original Message-
> From: Greg Stein [mailto:gst...@gmail.com]
> Sent: vrijdag 23 april 2010 0:57
> To: Hyrum K. Wright
> Cc: Subversion Development
> Subject: Re: Feature idea: user-configurable post-commit notifications
> 
> Write a hook script to do exactly that, and make it part of the
> standard release.
> 
> In some future release, after that script and its features stabilize,
> *then* we can consider placing into into the core code.
> 
> I fear the security aspects of something like you talk about: anybody
> with commit access getting the server to issue a POST request to any
> arbitrary URL *reachable* by the server? Oh ho ho... I can't even
> begin to describe how many alarms that would trip with security
> conscious administrators.

+1 on all of this.

Bert

RE: svn commit: r937475 - /subversion/trunk/subversion/libsvn_wc/util.c

[Bert Huijben]

> -Original Message-
> From: dan...@apache.org [mailto:dan...@apache.org]
> Sent: vrijdag 23 april 2010 21:12
> To: comm...@subversion.apache.org
> Subject: svn commit: r937475 -
> /subversion/trunk/subversion/libsvn_wc/util.c
> 
> Author: dannas
> Date: Fri Apr 23 19:11:34 2010
> New Revision: 937475
> 
> URL: http://svn.apache.org/viewvc?rev=937475&view=rev
> Log:
> * subversion/libsvn_wc/util.c
>   (svn_wc__status2_from_3): Add note about the need to do a proper
> conversion instead of just a cast.
> 
> Modified:
> subversion/trunk/subversion/libsvn_wc/util.c
> 
> Modified: subversion/trunk/subversion/libsvn_wc/util.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/util
> .c?rev=937475&r1=937474&r2=937475&view=diff
> ===
> ===
> --- subversion/trunk/subversion/libsvn_wc/util.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/util.c Fri Apr 23 19:11:34
> 2010
> @@ -548,5 +548,7 @@ svn_wc_status2_t *
>  svn_wc__status2_from_3(const svn_wc_status3_t *status,
> apr_pool_t *scratch_pool)
>  {
> +  /* ### As of r937468, status2_t and status3_t are no longer
> identical. We
> +   * ### need to extend this function to properly do the conversion.
> */
>return (svn_wc_status2_t *) svn_wc_dup_status3(status,
> scratch_pool);
>  }

I think you should put some assertion in here instead of letting the caller 
crash on invalid data. Or better yet, fix the conversion function while 
changing the structure

The structures are unrelated now and you removed the best testcase for the 
wrapper.

Bert 

RE: SVN Update does not add new files

[Bert Huijben]

> -Original Message-
> From: Martin Cerny [mailto:cern...@gmail.com]
> Sent: zaterdag 24 april 2010 12:27
> To: dev
> Subject: SVN Update does not add new files
> 
> Hi,
> I'm experiencing a bug with SVN.
> I have a branch of our main project. I did some work on it in school and
then
> wanted to update the status of my working copy at home. The files that
> were changed were correctly reflected in the working copy, but new files
> that I added at school were not added to my working copy at home. I can
see
> the files in correct place while browsing the repository with TortoiseSVN.
I
> tried updating both with TortoiseSVN 1.6.6, and via NetBeans plugin using
> svn version 1.6.6 and then with version 1.6.11 to no avail. Switching to
the
> trunk and then back to the branch fixes the problem.
> 
> The work at school was done using some older version of TortoiseSVN
> (1.6.2 I guess, but I am not certain)
> Our subversion server is version 1.6.6. I am sorry, but I can't grant you
acces
> to the repository itself, since it's a company server.

If I remember correctly TortoiseSVN 1.6.2 is based on Subversion 1.6.1,
which still had the bug that was fixed in Subversion 1.6.2:
  * set depth=infinity on 'svn add' items with restricted depth (r37607)

Another possible cause for this issue (but far less likely) is issue #3569
(http://subversion.tigris.org/issues/show_bug.cgi?id=3569): svn update
--depth  allows making a working copy incomplete

The first issue can be resolved by using 'svn update --set-depth infinity'
on the working copy (and is fixed in later clients). The second can only be
resolved by a new checkout and isn't fixed yet.

And of course it could be another issue, so if you have a different
reproduction recipe to show, please send it to this list. (In all cases I
heard it was one of these two reasons... and in most cases the working copy
was marked sparse long ago, without anybody noticing)

Bert

RE: [PATCH] Saving a few cycles, part 1/2

[Bert Huijben]

> -Original Message-
> From: Stefan Fuhrmann [mailto:stefanfuhrm...@alice-dsl.de]
> Sent: zondag 25 april 2010 16:52
> To: dev@subversion.apache.org
> Subject: [PATCH] Saving a few cycles, part 1/2
> 
> Hi devs,
> 
> further reducing my backlog of patches sitting in my working copy, this
and
> the next patch optimize code locally - shaving off cycles here and there.
The
> net effect is somewhere between 3 and 10 percent for repository access
(ls,
> export, etc.).
> 
> In this patch, I eliminated calls to memcpy for small copies as they are
> particularly expensive in the MS CRT.

Which CRT did you use for these measurements? (2005, 2008, 2010, Debug vs
Release and DLL vs Static?). Which compiler version? (Standard/Express or
Professional+). (I assume you use the normal Subversion build using .sln
files and not the TortoiseSVN scripts? Did you use the shared library builds
or a static build)?

Did you try enabling the intrinsincs for this method instead of using a
handcoded copy?

I'm pretty sure that the modern variants enable inlined optimized assembly
for memcpy in release mode (and certainly if you ask for that using the
right #pragma), but enabling profiling on the linker or advanced debugging
options will probably disable all that. I would be extremely surprised if
this optimized assembler is measurable slower than the CRT on other OSs as
copying memory is a pretty elementary operation.
(But I'm pretty certain that the debug CRT with variable usage and memory
usage tracking is slower than the other CRT's.. But that is why we deliver
applications in release mode)

In my performance measurements the only case where I saw a memcpy turn up
was in the apr pool functions. (This was while using a profiler that didn't
touch the code (just timed measure points) and tracing on optimized release
mode binaries)

Bert
> 
> -- Stefan^2.
> 
> [[[
> Eliminate memcpy from critical paths during reading data from the
> repository.
> 
> * subversion/libsvn_delta/text_delta.c
>   (svn_txdelta_apply_instructions): replace memcpy
>   for small amounts of data; optimize overlapping
>   copies; optimize 'buffer full' detection
> 
> * subversion/libsvn_subr/svn_string.c
>   (svn_stringbuf_appendbytes): replace memcpy
>   with specialized code when adding single chars.
> ]]]

RE: [PATCH] Saving a few cycles, part 1/2

[Bert Huijben]

> -Original Message-
> From: Stefan Fuhrmann [mailto:stefanfuhrm...@alice-dsl.de]
> Sent: zondag 25 april 2010 16:52
> To: dev@subversion.apache.org
> Subject: [PATCH] Saving a few cycles, part 1/2
> 
> Hi devs,
> 
> further reducing my backlog of patches sitting in my
> working copy, this and the next patch optimize code
> locally - shaving off cycles here and there. The net
> effect is somewhere between 3 and 10 percent
> for repository access (ls, export, etc.).
> 
> In this patch, I eliminated calls to memcpy for small
> copies as they are particularly expensive in the MS CRT.

If you don't use the intrinsincs (or IA-64), this is the sourcecode from the
MS CRT in VS2010.
(See %PROGAMFILES%\Microsoft Visual Studio 10.0\VC\crt\src\memcpy.c)

void * __cdecl memcpy (
void * dst,
const void * src,
size_t count
)
{
void * ret = dst;

/*
 * copy from lower addresses to higher addresses
 */
while (count--) {
*(char *)dst = *(char *)src;
dst = (char *)dst + 1;
src = (char *)src + 1;
}

return(ret);
}

Where would this be slower than the code you tried to replace it with?

Are you sure the overhead isn't in calling the C runtime in a separate DLL?

Bert

RE: svn looses history when moving directories

[Bert Huijben]

> -Original Message-
> From: Mark Phippard [mailto:mphipp...@collab.net]
> Sent: maandag 26 april 2010 16:03
> To: CollabNet Subversion Client
> Subject: Re: svn looses history when moving directories
> 
> On 4/26/10 9:42 AM, "Piet-Hein Peeters" 
> wrote:
> 
> > That's still not the same usecase. My usecase has one file and one
directory
> > move:
> > $ svn mkdir B
> > $ svn mv A/foo.c B <-- file move
> > $ svn info B/foo.c
> > $ svn mv B C <-- directory move
> > $ svn info C/foo.c
> 
> OK yes I see this too.
> 
> Prior to 1.5 you could not even do this.  The second move would just fail.
> You seem to be implying there is some new bug, in terms of your reference
> to
> 1.6.6, but from my recollection of when the developers agreed to relax the
> restriction in 1.5, this was the compromise.  It could allow the second
move
> but it had to convert the file into a plain Add to do so.
> 
> Is there a version of SVN where this worked?

I just added a regression test on this interesting case in Subversion trunk
(r93807) to make sure we get this fixed with the WC-NG work. 

I have no idea how hard it would be to fix this case for 1.6, but I would
certainly see this as a showstopper for the WC-NG store that was designed to
make recording operations like these much easier.

Bert

RE: svn commit: r938274 - /subversion/trunk/subversion/libsvn_wc/props.c

[Bert Huijben]

What about the case where that file already exists and another name is used?

Bert Huijben (mobile phone)

- Oorspronkelijk bericht -
Van: gst...@apache.org
Verzonden: dinsdag 27 april 2010 2:09
Aan: comm...@subversion.apache.org
Onderwerp: svn commit: r938274 - /subversion/trunk/subversion/libsvn_wc/props.c

Author: gstein
Date: Tue Apr 27 00:09:27 2010
New Revision: 938274

URL: http://svn.apache.org/viewvc?rev=938274&view=rev
Log:
Followup to r938268: trying to detect whether the rejects applied to the
directory or a child, the check was goofed. Gotta add the file extension.

* subversion/libsvn_wc/props.c:
  (svn_wc__get_prejfile_abspath): include the extension on the filename

Modified:
subversion/trunk/subversion/libsvn_wc/props.c

Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=938274&r1=938273&r2=938274&view=diff
==
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Tue Apr 27 00:09:27 2010
@@ -287,10 +287,12 @@ svn_wc__get_prejfile_abspath(const char 
 
   if (cd->kind == svn_wc_conflict_kind_property)
 {
-  if (strcmp(cd->their_file, SVN_WC__THIS_DIR_PREJ) == 0)
-*prejfile_abspath = svn_dirent_join(local_abspath,
-SVN_WC__THIS_DIR_PREJ,
-result_pool);
+  if (strcmp(cd->their_file,
+ SVN_WC__THIS_DIR_PREJ SVN_WC__PROP_REJ_EXT) == 0)
+*prejfile_abspath = svn_dirent_join(
+  local_abspath,
+  SVN_WC__THIS_DIR_PREJ SVN_WC__PROP_REJ_EXT,
+  result_pool);
   else
 *prejfile_abspath = svn_dirent_join(
   svn_dirent_dirname(local_abspath,

RE: [PATCH] Saving a few cycles, part 1/2

[Bert Huijben]

> -Original Message-
> From: Stefan Fuhrmann [mailto:stefanfuhrm...@alice-dsl.de]
> Sent: dinsdag 27 april 2010 1:10
> To: Bert Huijben; dev@subversion.apache.org
> Subject: Re: [PATCH] Saving a few cycles, part 1/2
> 
> Bert Huijben wrote:
> >
> >> -Original Message-
> >> From: Stefan Fuhrmann [mailto:stefanfuhrm...@alice-dsl.de]
> >> In this patch, I eliminated calls to memcpy for small copies as they
are
> >> particularly expensive in the MS CRT.
> >>
> >
> > Which CRT did you use for these measurements? (2005, 2008, 2010, Debug
> vs
> > Release and DLL vs Static?). Which compiler version? (Standard/Express
or
> > Professional+). (I assume you use the normal Subversion build using .sln
> > files and not the TortoiseSVN scripts? Did you use the shared library
builds
> > or a static build)?
> >
> VSTS2008 Developer Edition. Release build (am I an Amateur?!)
> TSVN build scripts which set /Ox (global opt, intrinsics, omit frame
> pointers, ...)
> > Did you try enabling the intrinsincs for this method instead of using a
> > handcoded copy?
> >
> 
> Yes, but it does not help in this case: memset will use intrinsics
> only for short (<48 bytes on x86) _fixed-size_  buffers. memcpy
> will use intrinsics for _fixed-size_ buffers only, but seemingly with
> no size limit.

But did you try a non-shared library build.

If you use the C runtime as a shared library things like using fastcall
instead of __cdecl or full program optimization don't matter as you don't
change msvcr90.dll (or a later version) in your build. The overhead of
calling a function in a DLL is probably bigger than the thing you are trying
to accomplish by handcoding your memcpy().

In your first mail you said " In this patch, I eliminated calls to memcpy
for small
copies as they are particularly expensive in the MS CRT."

Did you compare it to other toolchains?
And did you compare it to a completely static build without referencing to
msvcr90.dll?

Where these functions on other toolchains compiled into the binary or also
in an external dll?

When comparing 7 byte buffer copies, things like doing an indirect call for
a library function have a much bigger impact than shaving off a few
assembler instructions of the loop itself, so maybe just passing /MT instead
of /MD to the compiler makes the same difference. (It will certainly help on
the full optimization)

Looking at the TortoiseSVN build and my local set of binaries I see that it
uses MSVCR90.DLL from most of its libraries, and it at least uses memcpy()
from its dll in a few code paths.

Bert

RE: svn commit: r939150 - /subversion/trunk/subversion/libsvn_client/merge.c

[Bert Huijben]

> -Original Message-
> From: ne...@apache.org [mailto:ne...@apache.org]
> Sent: donderdag 29 april 2010 2:49
> To: comm...@subversion.apache.org
> Subject: svn commit: r939150 -
> /subversion/trunk/subversion/libsvn_client/merge.c
> 
> Author: neels
> Date: Thu Apr 29 00:49:16 2010
> New Revision: 939150
> 
> URL: http://svn.apache.org/viewvc?rev=939150&view=rev
> Log:
> Another entry_t down.
> 
> * subversion/libsvn_client/merge.c
>   (obstructed_or_missing):
> Use wc-ng API instead of svn_wc__get_entry_versioned(), and much
> simplify
> this function in the process. Untangle, rinse and dry.
>   (node_kind_working, node_kind_on_disk):
> Remove these ghastly two functions and incorporate into
> obstructed_or_missing(), the way it should always have been.

This patch breaks
FAIL:  merge_authz_tests.py 1: skipped paths get overriding mergeinfo
(which is automatically skipped on ra_local)

Most likely your new check doesn't work correctly for 'absent' nodes.
I would recommend also checking the behavior for excluded node, as our test 
suite doesn't test those as often as it should.
(Nice effect is that you can check excluded on ra_local, but can't check absent)

Bert

RE: [PATCH v2] Saving a few cycles, part 3/3

[Bert Huijben]

> -Original Message-
> From: Stefan Fuhrmann [mailto:stefanfuhrm...@alice-dsl.de]
> Sent: zaterdag 1 mei 2010 19:03
> To: dev@subversion.apache.org
> Subject: [PATCH v2] Saving a few cycles, part 3/3
> 
> Hi devs,
> 
> I reworked the patch set according to the feedback I got
> here on this list. This is what changed in this last part:
> 
> * add rationales to the commentary
> * bring svndiff.c changes closer to the original code
> * use svn_ctype_* functions in checksum parser
> 
> -- Stefan^2.
> 
> [[[
> Various local optimizations. These opportunities became
> visible after significantly optimizing other parts of svn.  The
> total savings for a 'svn export' is almost 3 percent on top
> of the previous.
> 
> The largest savings can be attributed to the svndiff.c
> changes (~1.5%) and the checksum parser (~1%).
> 
> * subversion/include/svn_delta.h
>   (enum svn_delta_action): ensure that these definitions
>   will always have these values, even if new definitions
>   were added

This part is not necessary. These values are part of our public API
compatibility promise and that makes the values static for 1.X. 

The C compiler defines the same values... and if it didn't you are not
allowed to change the values (for the same binary compatibility reasons).

In the rest of your patch you can use the enum constants in some places
instead of the magic values to increase some readability. (But I think the
old code didn't do that either).

> 
> * subversion/libsvn_delta/compose_delta.c
>   (search_offset_index): use size_t to index memory.
>   This is mainly a consistency fix but may actually result
>   in slightly higher performance due to fewer conversions.

s/size_t/apr_size_t/

And it can also cause a slowdown. But I like to see apr_size_t anyway.

>   (copy_source_ops): dito; optimize a common case
> 
> * subversion/libsvn_delta/svndiff.c
>   (decode_file_offset, decode_size): use slightly more
>   efficient formulations
>   (decode_instruction): directly map action codes -
>   made possible by defining the enum values

The enum values were already defined so that doesn't change anything (see
above: they can't change anyway). 

Optimizing a % operator for values of 1 and 2 looks like code obfuscation to
me, unless you can show me some numbers that it actually makes a difference.
I would assume that the processor optimizes that case itself.

> 
> * subversion/libsvn_subr/checksum.c
>   (svn_checksum_parse_hex): eliminate calls to locale-
>   aware CRT functions. At least with MS compilers,
>   these are very expensive.

Yes, these call into the OS-libraries on Windows.. Avoiding these helps *a
lot*.

> * subversion/libsvn_subr/stream.c
>   (stream_readline): numbytes is invariant until EOF
> 
> patch by stefanfuhrmann < at > alice-dsl.de
> ]]]

Bert

RE: svn commit: r940898 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/libsvn_wc/pristine-store-test.c

[Bert Huijben]

> -Original Message-
> From: julianf...@apache.org [mailto:julianf...@apache.org]
> Sent: dinsdag 4 mei 2010 17:14
> To: comm...@subversion.apache.org
> Subject: svn commit: r940898 - in /subversion/trunk/subversion:
> libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h
> tests/libsvn_wc/pristine-store-test.c
> 
> Author: julianfoad
> Date: Tue May  4 15:14:13 2010
> New Revision: 940898
> 
> URL: http://svn.apache.org/viewvc?rev=940898&view=rev
> Log:
> Add a function for deleting unreferenced pristine text files.
> 
> * subversion/libsvn_wc/wc_db.c,
>   subversion/libsvn_wc/wc_db.h
>   (svn_wc__db_pristine_remove): New function.
> 
> * subversion/libsvn_wc/wc-queries.sql
>   (STMT_SELECT_ANY_PRISTINE_REFERENCE): New query.

You also added STMT_DELETE_PRISTINE in this patch.

Bert

RE: another hole in tree-conflicts! -- was: Re: svn commit: r940111 - in /subversion/trunk/subversion: include/private/svn_wc_private.h include/svn_wc.h libsvn_wc/node.c libsvn_wc/status.c svn/cl.h

[Bert Huijben]

> -Original Message-
> From: Neels J Hofmeyr [mailto:ne...@elego.de]
> Sent: woensdag 5 mei 2010 11:37
> To: dev@subversion.apache.org; Julian Foad; Stefan Sperling; Stephen
> Butler
> Subject: another hole in tree-conflicts! -- was: Re: svn commit:
> r940111 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h include/svn_wc.h libsvn_wc/node.c
> libsvn_wc/status.c svn/cl.h svn/status-cmd.c svn/status.c
> 
> On 05/03/2010 10:25 PM, Daniel Näslund wrote:
> > On Mon, May 03, 2010 at 03:07:49PM +0200, Neels J Hofmeyr wrote:
> [...]
> >>>stat->lock_creation_date = 0;
> >>> +  stat->conflicted = (tree_conflict != NULL);
> >>
> >> I don't understand -- what about prop and text conflicts? Below,
> >> ->conflicted stands for all three of them.
> >> (If this is correct, I guess it needs a comment)
> >
> > Added a comment in r940600. AFAIK, the only scenario where we can get
> a
> > conflict on an unversioned node is for an incoming delete to a
> locally
> > deleted path. (It will be troublesome to fit that case into the idea
> of
> > storing tree conflict info on the node instead of its parent).
> 
> ah, ok, that makes sense. Shouldn't be trouble to store such tree-
> conflict
> in the ng-ACTUAL tree? ...but, thinking aloud a little:
> 
> The entry can't be NULL for a schedule-deleted node (asked for with
> show_hidden == TRUE). Analogously, the wc-ng tables do have both ng-
> BASE and
> ng-WORKING nodes for a locally deleted path.

This is the case where a node was deleted locally (schedule delete) and
deleted remotely. 

After the update we couldn't leave it schedule delete as the entry was
removed from the repository, so the complete node was removed in 1.6 and a
tree conflict recorded in the parent *entry*.
(All cases where we have a node would fall through a different codepath in
the status walk)


We discussed this scenario a few weeks ago on irc, and there are two ways to
recording this in WC-NG:
1. Allow an ACTUAL_NODE record for nodes without BASE_NODE and WORKING_NODE
2. Keep a NOT-PRESENT BASE_NODE record for this case

I recommended option 2, as it doesn't introduce another status just for
recording this conflict type which we would have to check everywhere.
(Adding another status would require every loop over the result of
svn_wc__db_read_children() to check for this status).

> So, if there is no metadata in the WC on the node, there can't be *any*
> (tree-)conflict on it! Right? Let's see:
> 
> Another case: obstructed; there is no metadata in the WC, but a
> file/folder
> sits at a path. Say update wants to put something at that path: it'll
> create
> a base node (above theory holds -- TC needs to have metadata).

In WC-1.0 we record this on the parent, without an entry.

In WC-NG we can create the node, but with an 'obstructed' conflict on it.
(So we can allow the 'svn update to continue')


The rest of your mail talks about how merge handles this conflict, which is
a less interesting case for the wc-ng work where I'm looking at as it is a
client feature. (But it might require recording a conflict on a not-present
node anyway)

Bert

RE: svn commit: r941617 - in /subversion/trunk/subversion/libsvn_wc: wc-queries.sql wc_db.c wc_db.h

[Bert Huijben]

> -Original Message-
> From: julianf...@apache.org [mailto:julianf...@apache.org]
> Sent: donderdag 6 mei 2010 10:53
> To: comm...@subversion.apache.org
> Subject: svn commit: r941617 - in /subversion/trunk/subversion/libsvn_wc:
> wc-queries.sql wc_db.c wc_db.h
> 
> Author: julianfoad
> Date: Thu May  6 08:53:11 2010
> New Revision: 941617
> 
> URL: http://svn.apache.org/viewvc?rev=941617&view=rev
> Log:
> Add a function for looking up a pristine text's SHA-1 checksum from its MD-5
> checksum - the inverse of svn_wc__db_pristine_get_md5().
> 
> * subversion/libsvn_wc/wc_db.c,
>   subversion/libsvn_wc/wc_db.h
>   (svn_wc__db_pristine_get_md5): Rename parameters in the function
> prototype
> to match the function definition.
>   (svn_wc__db_pristine_get_sha1): New function.

I think you should make this a '_temp_' function as we should not do this 
conversion in 1.7.0. (But it is very useful in the intermediate state where we 
are now). 
Or is it needed for the commit processing via the old API?

Bert 

RE: conflict-storage, and filenames

[Bert Huijben]

> -Original Message-
> From: Stefan Sperling [mailto:s...@elego.de]
> Sent: zondag 9 mei 2010 15:43
> To: Greg Stein
> Cc: dev@subversion.apache.org
> Subject: Re: conflict-storage, and filenames
> 
> On Sat, May 08, 2010 at 11:15:31PM -0400, Greg Stein wrote:
> > Hey all,
> >
> > In wc-1, we record filenames where merge sources are stored, and
> where
> > property rejects are written. These are:
> >
> > entry->conflict_wrk, ->conflict_old, ->conflict_new, and ->prejfile
> >
> > These file names do not appear to have any corresponding values in
> the
> > proposed skels detailed in notes/wc-ng/conflict-storage.
> >
> > We cannot simply derive the filenames because there may be versioned
> > (or unversioned) content with the same names.
> 
> That problem must also exists in wc-1. What does wc-1 do if the files
> already exist?

It creates a file with a different (unique) name and it stores the name that
is used in the svn_wc_entry_t. (The code uses svn_io_open_uniquely_named()
to create the conflict marker file)

As suggested on IRC, I think it is just fine to store the basename of the
conflict filename in the skel as we don't use it in many places. We should
probably store a list of names per conflict type (and maybe allow the same
file to be used for multiple conflict types at the same time?).

We need the names when resolving the conflict and when we want to detect if
a conflict has been resolved by just removing the marker files (We currently
do this from some code paths), but not in performance critical functions as
status; we see that there are no conflicts with _read_info()'s conflicted
boolean and we see the conflict marker files as unversioned.

Another option is to still keep them in the separate columns where they are
now. (Shouldn't be much trouble if we want to store the checksums only in
their own columns, instead of duplicating them in the skels. But if I
remember correctly we do duplicate the values)

Bert

RE: conflict-storage, and filenames

[Bert Huijben]

> -Original Message-
> From: Stefan Sperling [mailto:s...@elego.de]
> Sent: maandag 10 mei 2010 0:05
> To: Greg Stein
> Cc: Bert Huijben; dev@subversion.apache.org
> Subject: Re: conflict-storage, and filenames
> 
> On Sun, May 09, 2010 at 04:54:17PM -0400, Greg Stein wrote:
> > On Sun, May 9, 2010 at 09:43, Stefan Sperling  wrote:
> > > Well, I've been under the impression that the names are currently
> > > 100% predictable. Is that not the case?
> >
> > Nope.
> >
> > $ svn add foo.c foo.c.left foo.c.1.left
> > $ # do something to create conflict file: foo.c.2.left
> > $ svn rm foo.c.1.left
> >
> > If we scanned for the "left" conflict file, we'd stop at foo.c.1.left
> > and never find the *real* one: foo.c.2.left
> >
> > Thus, we have to store the filename that was used.
> >
> > > Assuming the names are predictable, I don't see a need to record
> the names,
> > > so can you explain what you think would break by not recording
> them?
> > > What problem does it really cause for us, or for users?
> >
> > As Bert explained, we need to remove them when the user runs "svn
> > resolved". He also noted that (somtimes) it is possible manually
> > resolve a conflict by removing all the conflict files (a potentially
> > debatable feature).
> 
> I see. Then let's just add another field to the skel.
> I guess we can store this within the conflict-type-specific data?
> Storing the basename should be enough since we can assume the file
> will be put into the same directory as the conflicted file, right?

For text conflicts this would be the case, but for a property conflict on a
directory it would be harder to tell where the file is located. (What if the
directory is missing?). Greg suggested adding a wcroot-relative path on IRC.

(Timezone=CEST)
23:03 <@gstein> Bert: re: storing basenames vs relpaths... I think we'll
store relpaths,
23:03 <@gstein> and have a db function to convert to/from the relpaths,
23:03 <@gstein> because we also need that in the workqueue,
23:04 <@gstein> today, we store abspaths in the workqueue which isn't good
23:04 <@Bert> gstein: That will work, once we have a central db. But until
we reach that point the relpath is the basename
23:05 <@gstein> sure, but we should use the conversion functions rather than
just "basename", so that the code automatically switches correctly when we
move to single-db
23:06 <@Bert> (And I didn't know if you intended to make that info public
within libsvn_wc)
23:09 <@gstein> svn_wc__db_to_relpath() and from_relpath(), or somesuch
23:10 <@Bert> *nod*
23:11 <@gstein> not sure if those should be in svn_wc,
23:11 <@gstein> since we want svn_wc to use abspaths

Bert

RE: svn commit: r943445 - in /subversion/trunk/subversion: svn/cl.h svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout

[Bert Huijben]

> -Original Message-
> From: s...@apache.org [mailto:s...@apache.org]
> Sent: woensdag 12 mei 2010 12:59
> To: comm...@subversion.apache.org
> Subject: svn commit: r943445 - in /subversion/trunk/subversion: svn/cl.h
> svn/log-cmd.c svn/main.c
> tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
> 
> Author: stsp
> Date: Wed May 12 10:59:27 2010
> New Revision: 943445
> 
> URL: http://svn.apache.org/viewvc?rev=943445&view=rev
> Log:
> Add a new --show-diff option to svn log, fixing part of issue #2909.
> 
> svn log --show-diff makes the svn log command provide inline diff output.
> This is a fairly naive implementation, done entirely within the CLI client.
> A separate RA session is opened to retrieve the diff. It behaves more or
> less like a series of svn log; svn diff invocations. This means first and
> foremost that it is a bit slow. But it works.

Thanks for doing this in svn instead of changing the APIs for a CLI client 
specific feature. :)

Bert

Re: What are the plans for svn_wc_status3_t?

[Bert Huijben]

On Wed, May 12, 2010 at 3:10 PM, Daniel Näslund  wrote:
> Hi!
>
> I set out to remove the entry field in svn_wc_status3_t. The reasons was
> i) svn_wc_entry_t is deprecated and ii) fetching all the info contained
> in the entry field is slow and we want 'svn status' to be fast!
>
> So far this has been done:
> --
> * The revision information has been moved out to its own fields. The
>  idea is that those fields should be clearly defined, contrary to the
>  entry ones.
> * entry->changelist information is fetched with a node func in the
>  caller.

Or you can just read it with _read_info() with all other information

> * Added a field, status->conflicted. The caller is supposed to find out
>  what conflict it is.


> * Added a field status->versioned. The caller can use this one instead
>  of the presence of status->entry for determining if a path is
>  versioned.
>
> What are the plans for...
> ---
> * status->text_status. Should it be replaced with a boolean indicating
>  if there are text modifications?
> * status->prop_status. Should it be replaced with a boolean indicating
>  if there are props modifications?

I would keep these working, or replace them with a similar wc-ng
status, but keep the status if we also see a conflict or other high
priority status. (In status2_t you loose status_modified if you have a
conflict)

You call status to see if a node is locally modified, so you need this
info in one format or another

> * status->copied. It could be a bit expensive to calculate but only
>  returning status_added and have the API user determine if it's copied
>  feels like a leaky abstraction to me. My take is that all API users
>  wants to know if the node is copied.
> * status->switched. Everyone probably wants to know that too. We need
>  the url's to determine if the node is switched which brings me to...
> * status->url. It is not used in svn/status{,-cmd}.c. How many other
>  callers use it? If we keep status->switched, we still have to fetch
>  the url though.

If we have it for free we can just pass it to users. (I know it is
used by many clients).

You need the url of a node and its parent to determine if a node is
switched, so I would keep both in the structure.
(In AnkhSVN I'm really not that interested in switched.. I just want
to know if a node is commit/diff/revertable.)

You should also add a node kind to the status structure. (Status
without node kind requires looking at disk).
And probably depth too as you often need that to process the result.
(And you have both available anyway)

   Bert
> * svn_wc_status_kind. Should it be revved to map to
>  svn_wc__db_status_kind_t in some other way. We've already made the
>  svn_wc_status_conflicted alternative obsolute (atleast for
>  svn_wc_status3_t. I haven't checked the other uses of
>  svn_wc_status_kind).
>
>  Cheers,
>  Daniel

RE: First SVN performance data

[Bert Huijben]

> -Original Message-
> From: Stefan Fuhrmann [mailto:stefanfuhrm...@alice-dsl.de]
> Sent: woensdag 12 mei 2010 12:25
> To: d...@apr.apache.org
> Subject: First SVN performance data
> 
> Hi there,
> 
> as I promised, I'm going to conduct some in-depth analysis and
> comprehensive SVN performance testing.
> That is very time-consuming process.

I think this belongs on d...@subversion instead of d...@apr; moving to that
list.

> 
> However, it seems that many people have incorrect or outdated ideas about
> the current state of affairs.
> To add a bit more substance to the discussion, I like to present some
> preliminary data and not to wait until I collected all data I intend to.


> 
> Side note: Maybe, these numbers make it clearer why my patches should be
> committed after review.
> 
> Bottom line:
> * SVN servers tend to be CPU-limited
>   (we already observed that problem @ our company
>   with SVN 1.4)
> * packed repositories are ~20% faster than non-packed,
>   non-sharded
> * optimal file cache size is roughly /trunk size
>   (plus branch diffs, but that is yet to be quantified)
> * "cold" I/O from a low-latency source takes 2 .. 3 times
>   as long as from cached data
> * a fully patched 1.7 server is twice as fast as 1.6.9
> 
> "Export" has been chosen to eliminate problems with client-side w/c
> performance.

Note that this also eliminates delta transfers, which are very common in
these wc operations.  Subversion tries to optimize transfers by only
transferring binary updates, but you eliminated that.

I would assume that transferring full files as deltas is not the most
efficient way to transfer these full files. No surprises there. (And
transferring deltas will hopefully be faster too after this work)

In the performance measurements I did just before releasing 1.6 (on
checkout, update, merge) I found that most time was spend on client-io in
the working copy and on locking, but all of this was removed from your
testset as you are only testing the subversion fs (filesystem) and IO
performance.

While I'm certain that these components can use more than a bit of
optimization to enhance the scalability of Subversion, I'm not sure if these
are the true performance problems of subversion as noted in our user
feedback.

If you are looking at just 'svn export' you completely eliminate the
reporting phase from an update editor (the client->server reporting), which
is one of the major components of svn update and svn status --update. You
just leave the server->client data transfer, the in-memory property handling
and writing out the possibly translated files on the client. 

Michael Pilato and Hyrum Wright interviewed some enterprise users earlier
this year and wrote some reports which indicated that the network latency
and working copy performance were the true bottlenecks. If I look at
^/subversion/trunk/notes/feedback, I see checkout, log, merging as primary
performance issues and this matches the performance issues I see in my day
to day use of repositories on the other side of the world. Theoretically svn
checkout should be pretty similar to the svn export your testing, but
Subversion completely implements this as updating from r0 to the specified
revision. Most of the time on these operations is spend on the client
waiting for disk (libsvn_wc administration, moving and rewriting files) and
on network-IO caused by latency (not by throughput).

(On the Slik Windows buildbot, which runs all tests on a NTFS ramdrive, I
see that the tests are CPU bound these days... But that is after eliminating
all disk IO during the tests. When I run them on a normal workstation disk I
see different results)

> Please note that all measurements were taken in a true client/server
setup.
> You can achieve similar performance in low-latency broad-band networks.

Bert

> 
> -- Stefan^2.

RE: svn commit: r943986 - /subversion/trunk/subversion/libsvn_wc/status.c

[Bert Huijben]

> -Original Message-
> From: dan...@apache.org [mailto:dan...@apache.org]
> Sent: donderdag 13 mei 2010 21:32
> To: comm...@subversion.apache.org
> Subject: svn commit: r943986 -
> /subversion/trunk/subversion/libsvn_wc/status.c
> 
> Author: dannas
> Date: Thu May 13 19:32:11 2010
> New Revision: 943986
> 
> URL: http://svn.apache.org/viewvc?rev=943986&view=rev
> Log:
> Don't use svn_wc_entry for fetching an url.
> 
> * subversion/libsvn_wc/status.c
>   (assemble_status): Fetch the url with a node func. Use
> svn_wc__check_wc_root() for determining if a path is switched.
> 
> Modified:
> subversion/trunk/subversion/libsvn_wc/status.c
> 
> Modified: subversion/trunk/subversion/libsvn_wc/status.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/stat
> us.c?rev=943986&r1=943985&r2=943986&view=diff
> ==
> 
> --- subversion/trunk/subversion/libsvn_wc/status.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/status.c Thu May 13 19:32:11
> +++ 2010
> @@ -284,6 +284,7 @@ assemble_status(svn_wc_status3_t **statu
>svn_wc_status3_t *stat;
>svn_wc__db_status_t db_status;
>svn_wc__db_kind_t db_kind;
> +  const char *url;
>svn_boolean_t locked_p = FALSE;
>svn_boolean_t switched_p = FALSE;
>const svn_wc_conflict_description2_t *tree_conflict; @@ -320,25 +321,22
> @@ assemble_status(svn_wc_status3_t **statu
> &lock, db, local_abspath, result_pool,
> scratch_pool));
> 
> +  SVN_ERR(svn_wc__internal_node_get_url(&url, db, local_abspath,
> +result_pool, scratch_pool));
> +
>/** File externals are switched files, but they are not shown as
>such.  To be switched it must have both an URL and a parent with
> -  an URL, at the very least.  If this is the root folder on the
> -  (virtual) disk, entry and parent_entry will be equal. */
> +  an URL, at the very least. */
>if (entry->file_external_path)
>  {
>file_external_p = TRUE;
>  }
> -  else if (entry->url && parent_entry && parent_entry->url &&
> -   entry != parent_entry)
> +  else
>  {
> -  /* An item is switched if:
> - parent-url + basename(path) != entry->url  */
> -  switched_p = (strcmp(
> - svn_uri_join(parent_entry->url,
> -  svn_path_uri_encode(svn_dirent_basename(
> -local_abspath, NULL),
> -  scratch_pool),
> -  scratch_pool), entry->url) != 0);
> +  svn_boolean_t is_wc_root; /* Not used. */
> +
> +  SVN_ERR(svn_wc__check_wc_root(&is_wc_root, NULL, &switched_p,
> db,
> +local_abspath, scratch_pool));

You replace a few in memory operations with several database operations on the 
node and its parent(s) here. Was this intended? (I don't think this will make 
status faster). 
While walking the tree for status you know the url of the parent and of the 
node itself.

This check_wc_root function starts looking in the database for the url of the 
node.. and its parent (probably inherited, so looking further up)... and then 
does this same check for you.

Bert

>  }
> 
>/* Examine whether our directory metadata is present, and compensate
> @@ -603,7 +601,7 @@ assemble_status(svn_wc_status3_t **statu
>stat->file_external = file_external_p;
>stat->copied = entry->copied;
>stat->repos_lock = repos_lock;
> -  stat->url = (entry->url ? entry->url : NULL);
> +  stat->url = url;
>stat->revision = revision;
>stat->changed_rev = changed_rev;
>stat->changed_author = changed_author;
> 

RE: problems with serf

[Bert Huijben]

> -Original Message-
> From: Mark Phippard [mailto:markp...@gmail.com]
> Sent: zondag 16 mei 2010 18:19
> To: Greg Stein
> Cc: Stefan Küng; Subversion Development
> Subject: Re: problems with serf
> 
> On Sun, May 16, 2010 at 11:29 AM, Greg Stein  wrote:
> > I'll take a look at your stack traces. Thanks!
> >
> > It is very strange that this happens for you. I've been using serf as
> > my only connection for the past couple years. I specifically disable
> > neon support. It's been working just fine for me, so there is
> > something on your system or about how TortoiseSVN interacts with the
> > libraries that triggers it.
> 
> It is also possible it is Serf on Windows, and not specifically
> TortoiseSVN.  I do not know if Bert uses Serf regularly or not.  I
> seem to recall Paul mentioning he does not use it because of crashes.
> I do not know if any of us that use Windows are even using trunk
> regularly.  The performance difference on Windows was too great to use
> it for any real day to day work.

The Slik buildbot runs the serf tests on Windows (The svn-slik-w2k3-x64-ra
bot checks svn:// and serf http://), but running the tests doesn't really
test actual network conditions. 
(I can't checkout a normal Subversion trunk over HTTP using serf when the
TCG isa server is in its way (I get 404 errors). But I see no issues when
doing the same thing from home)

Bert

RE: [PATCH] readable error messages on non-ASCII systems, v2

[Bert Huijben]

> -Original Message-
> From: Julian Foad [mailto:julian.f...@wandisco.com]
> Sent: maandag 17 mei 2010 11:25
> To: Greg Ames
> Cc: Subversion Dev
> Subject: Re: [PATCH] readable error messages on non-ASCII systems, v2
> 
> (Cc'ing the dev@ list.)
> 
> On Fri, 2010-05-14, Greg Ames wrote:
> > Fri, May 14, 2010, Julian Foad wrote:
> > > In an attempt both to understand and to minimize the patch: would you
> > > get the same result by converting 'err->message' from native encoding to
> > > UTF-8 and then printing it in the way that it currently is printed?
> >
> > I had to adjust the patch to fit 1.5.6 (no SQLite here).  Then
> > svn_utf_* reported that there were non-UTF-8 characters at the
> > beginning of the message.  This time it was the "svn: " prefix passed
> > to svn_cmdline_fprintf a few lines down.  I changed the patch to also
> > convert the prefix input variable to UTF-8 and pass that to
> > svn_cmdline_fprintf, and I see readable error messages again.
> 
> Hi Greg.  Great - it sounds like that gives you a less invasive solution
> to that particular bit.  Let us know how you get on.

Googling for z/OS and Ascii shows that there are some z/OS C compilers that 
allow compiling programs using ASCII literals.
(E.g. http://www-01.ibm.com/software/awdtools/czos/features/czosv1r2.html: See 
'ASCII Support')

Did you try using these compiler flags instead of trying to fix the source code?

I'm afraid that you will only find more and more places where we assume that 
our literals use ASCII, so if I would have to fix Subversion I would look into 
using some automated tool. (Another thing I thought about was using some kind 
of C preprocessor before compiling, which translates the strings to utf-8 using 
some escaping mechanism)

I think our output handling is mostly separated to allow converting from utf-8 
to the native encoding, so maybe applying a few compiler flags can help you 
more than patching the sourcecode.

Bert
> 
> - Julian
> 

RE: svn commit: r945350 - /subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py

[Bert Huijben]

> -Original Message-
> From: pbu...@apache.org [mailto:pbu...@apache.org]
> Sent: maandag 17 mei 2010 22:34
> To: comm...@subversion.apache.org
> Subject: svn commit: r945350 -
> /subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py
> 
> Author: pburba
> Date: Mon May 17 20:34:12 2010
> New Revision: 945350
> 
> URL: http://svn.apache.org/viewvc?rev=945350&view=rev
> Log:
> Add a new merge_authz test for reintegrate merges and issue #3242.
> 
> See http://subversion.tigris.org/issues/show_bug.cgi?id=3242#desc78
> 
> * subversion/tests/cmdline/merge_authz_tests.py
> 
>   (reintegrate_fails_if_no_root_access): New.
> 
>   (test_list): Add reintegrate_fails_if_no_root_access, set as XFail.

It looks like this test XPasses for ra_serf:

See the Windows bot:
http://ci.apache.org/builders/svn-slik-w2k3-x64-ra/builds/33/steps/Test%20fsfs%2Bsvn/logs/stdio

(But it doesn't XFail on the mac serf bot?!)

Can somebody explain this behavior?

Bert

RE: svn commit: r945350 - /subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py

[Bert Huijben]

> -Original Message-
> From: Bert Huijben [mailto:b...@qqmail.nl]
> Sent: dinsdag 18 mei 2010 9:03
> To: pbu...@apache.org
> Cc: dev@subversion.apache.org
> Subject: RE: svn commit: r945350 -
> /subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py
> 
> > -Original Message-
> > From: pbu...@apache.org [mailto:pbu...@apache.org]
> > Sent: maandag 17 mei 2010 22:34
> > To: comm...@subversion.apache.org
> > Subject: svn commit: r945350 -
> > /subversion/trunk/subversion/tests/cmdline/merge_authz_tests.py
> >
> > Author: pburba
> > Date: Mon May 17 20:34:12 2010
> > New Revision: 945350
> >
> > URL: http://svn.apache.org/viewvc?rev=945350&view=rev
> > Log:
> > Add a new merge_authz test for reintegrate merges and issue #3242.
> >
> > See http://subversion.tigris.org/issues/show_bug.cgi?id=3242#desc78
> >
> > * subversion/tests/cmdline/merge_authz_tests.py
> >
> >   (reintegrate_fails_if_no_root_access): New.
> >
> >   (test_list): Add reintegrate_fails_if_no_root_access, set as XFail.
> 
> It looks like this test XPasses for ra_serf:
> 
> See the Windows bot:
> http://ci.apache.org/builders/svn-slik-w2k3-x64-
> ra/builds/33/steps/Test%20fsfs%2Bsvn/logs/stdio
> 
> (But it doesn't XFail on the mac serf bot?!)
> 
> Can somebody explain this behavior?

Taking a cup of coffee provided the answer: It is broken only for DAV, not for 
svn://.

> 
>   Bert

RE: [WIP] Use repos_root_url and repos_relpath in the status code.

[Bert Huijben]

> -Original Message-
> From: Daniel Näslund [mailto:dan...@longitudo.com]
> Sent: zondag 16 mei 2010 23:24
> To: dev@subversion.apache.org
> Subject: [WIP] Use repos_root_url and repos_relpath in the status code.
> 
> Hi!
> 
> There's a lot of parameteter tracking in this patch. Basically it's all
> about passing down the url arguments to assemble_status().
> 
> The goal is that we should be able to remove the entry and parent_entry
> fields in a follow-up and be able to use the parent_relpath when we want
> to detect switches but also for determining the toplevel op_root of a
> copy (we may have a copy below another copy with mixed revs).
> 
> Sending it in to see if anyone has any objections. I don't feel
> comfortable committing it without someone more experienced giving it a
> look.
> 
> [[[
> First step in the move to using repos_root_url and repos_relpaths for
> describing urls in the status code.
> 
> The idea is to reuse the parents repos_relpath when detecting if a node is
> switched instead of doing an extra scan for each node.  Since we're doing
a
> depth first traversal of the wc, the parent has already been visited.
> Hopefully we will save some read calls.
> 
> We still depend on the url field but the plan is to remove it.
> 
> * subversion/include/svn_wc.h
>   (svn_wc_status3_t): Add new fields 'repos_root_url' and 'repos_relpath'.
> 
> * subversion/libsvn_wc/status.c
>   (assemble_status): Add new parameters. Go back to using the
> previous way of detecting a switched path; simply comparing the
> repos_relpath with the parent_repos_relpath + basename(path).
>   (send_status_structure): Add parameter 'parent_repos_root_url' and
> 'parent_repos_relpath.
>   (handle_dir_entry): Add parameter 'dir_repos_root_url' and
> 'dir_repos_relpath'.
>   (internal_status): Fetch the parent_repos_root_url and
> parent_repos_relpath from the db. This function
> is called on the anchor paths.
>   (get_dir_status): Add parameter 'parent_repos_root_url' and
> 'parent_repos_relpath.
>   (svn_wc_dup_status3): Duplicate 'repos_root_url' and 'repos_relpath'.
> ]]]

> Index: subversion/include/svn_wc.h
> ===
> --- subversion/include/svn_wc.h   (revision 944886)
> +++ subversion/include/svn_wc.h   (working copy)
> @@ -3647,6 +3647,13 @@ typedef struct svn_wc_status3_t
>/** Which changelist this item is part of, or NULL if not part of any.
*/
>const char *changelist;
>  
> +  /** The leading part of the url, not including the wc root and
subsequent
> +   * paths. */
> +  const char *repos_root_url;
> +
> +  /** The path relative to the wc root. */
> +  const char *repos_relpath;
> +
>/* NOTE! Please update svn_wc_dup_status3() when adding new fields
here. */
>  } svn_wc_status3_t;
>  
> Index: subversion/libsvn_wc/status.c
> ===
> --- subversion/libsvn_wc/status.c (revision 944886)
> +++ subversion/libsvn_wc/status.c (working copy)
> @@ -273,6 +273,8 @@ assemble_status(svn_wc_status3_t **status,
>  const char *local_abspath,
>  const svn_wc_entry_t *entry,
>  const svn_wc_entry_t *parent_entry,
> +const char *parent_repos_root_url,
> +const char *parent_repos_relpath,
>  svn_node_kind_t path_kind,
>  svn_boolean_t path_special,
>  svn_boolean_t get_all,
> @@ -284,6 +286,8 @@ assemble_status(svn_wc_status3_t **status,
>svn_wc_status3_t *stat;
>svn_wc__db_status_t db_status;
>svn_wc__db_kind_t db_kind;
> +  const char *repos_relpath;
> +  const char *repos_root_url;
>const char *url;
>svn_boolean_t locked_p = FALSE;
>svn_boolean_t switched_p = FALSE;
> @@ -313,16 +317,20 @@ assemble_status(svn_wc_status3_t **status,
>SVN_ERR(svn_wc__db_op_read_tree_conflict(&tree_conflict, db,
local_abspath,
> scratch_pool, scratch_pool));
>  
> -  SVN_ERR(svn_wc__db_read_info(&db_status, &db_kind, &revision, NULL,
NULL,
> -   NULL, &changed_rev, &changed_date,
> +  SVN_ERR(svn_wc__db_read_info(&db_status, &db_kind, &revision,
> +   &repos_relpath, &repos_root_url, NULL,
> +   &changed_rev, &changed_date,
> &changed_author, NULL, NULL, NULL, NULL,
> NULL, &changelist, NULL, NULL, NULL, NULL,
> NULL, NULL, &base_shadowed, &conflicted,
> &lock, db, local_abspath, result_pool,
> scratch_pool));
>  
> +  /* ### Temporary until we've revved svn_wc_status3_t to use
> +   * ### repos_{root_url,relpath} */
>SVN_ERR(svn_wc__internal_node_get_url(&url, db, local_abspath,
>  re

RE: svn trunk r945350: PASS (x64-ubuntu gcc)

[Bert Huijben]

> -Original Message-
> From: Joe Swatosh [mailto:joe.swat...@gmail.com]
> Sent: woensdag 19 mei 2010 8:33
> To: dev@subversion.apache.org
> Subject: Re: svn trunk r945350: PASS (x64-ubuntu gcc)
> 
> Is the Ubuntu Buildbot the only one still sending to
> svn-break...@subversion.tigris.org?  Is there an apache list somewhere
> for watching buildbot results? (None listed on
> http://subversion.apache.org/mailing-lists.html).

The buildbots have been moved to the ASF master: See
http://tinyurl.com/33pmv2z (shortcut for http://ci.apache.org/waterfall
config) for their current status.

The notifications for the ASF buildbot master are sent to the
notificati...@subversion.apache.org mailinglist.

Bert

RE: [Patch] RE: Windows drive letter check fails on lower case current working drive

[Bert Huijben]

> -Original Message-
> From: Bert Huijben [mailto:b...@qqmail.nl]
> Sent: woensdag 20 januari 2010 11:31
> To: 'Branko Čibej'; 'William A. Rowe Jr.'
> Cc: d...@apr.apache.org; 'Philip Martin'; 'Bert Huijben';
> dev@subversion.apache.org
> Subject: RE: [Patch] RE: Windows drive letter check fails on lower case
> current working drive
> 
> > -Original Message-
> > From: Branko Čibej [mailto:br...@xbc.nu]
> > Sent: woensdag 20 januari 2010 10:17
> > To: William A. Rowe Jr.
> > Cc: Bert Huijben; d...@apr.apache.org; 'Philip Martin'; 'Bert
> Huijben';
> > dev@subversion.apache.org
> > Subject: Re: [Patch] RE: Windows drive letter check fails on lower
> case
> > current working drive
> >
> > William A. Rowe Jr. wrote:
> > > On 1/19/2010 5:19 AM, Bert Huijben wrote:
> > >
> > >> The patch was written on the 1.4.x branch but I svn switch'ed it
> to trunk
> > for easy application.
> > >>
> > >
> > > I would suggest one bit of alternate code that is a bit more
> condensed, any
> > > concern?;
> > >
> > > +static int same_drive(const char *path1, const char *path2)
> > > +{
> > > +if ((path1[0] < 'A' || (path1[0] > 'Z' && path1[0] < 'a') ||
> path1[0] > 'Z')
> > > + || (path2[0] < 'A' || (path2[0] > 'Z' && path2[0] < 'a') ||
> path2[0] > 'Z')
> > > + || path1[1] != ':' || path2[1] != ':')
> > > +return 0;
> > >
> > > +/* Once in the domain of ASCII alpha, compare these case
> insensitive */
> > > +return ((path1[0] & 0x1f) == (path2[0] & 0x1f));
> > > +}
> > >
> > Whatever you do, those last Z's should be z's or it all breaks.
> 
> No, problem... I initially wrote a more condensed form myself and just
> made it a bit more readable later.
> 
> But please fix that 'Z' ;)

Ping,

Subversion is trying to get Subversion 1.7 test releases out by this summer, 
and we really need this fixed (or a workaround) before this release.

Bert

RE: svn commit: r946661 - /subversion/trunk/subversion/libsvn_wc/adm_crawler.c

[Bert Huijben]

> -Original Message-
> From: s...@apache.org [mailto:s...@apache.org]
> Sent: donderdag 20 mei 2010 17:11
> To: comm...@subversion.apache.org
> Subject: svn commit: r946661 -
> /subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> 
> Author: stsp
> Date: Thu May 20 15:11:07 2010
> New Revision: 946661
> 
> URL: http://svn.apache.org/viewvc?rev=946661&view=rev
> Log:
> Fix issue #2267, "support uncommitted svn:externals properties".
> 
> * subversion/libsvn_wc/adm_crawler.c
>   (read_traversal_info): Rename to ...
>   (read_externals_info): ... this. We've been using an external_func
>callback instead of a traversal info for some time.
>   (report_revisions_and_depths): Rename local variable CHILDREN to
>BASE_CHILDREN for clarity (the children come from the BASE tree).
>If the caller provided an EXTERNAL_FUNC callback, check locally
>added directories for svn:externals as well and pass them to the
>callback. The caller will then pull externals into the added directory.

Hi,

I'm not sure if this really fixes this issue for the common use cases. (And it 
introduces libsvn_client specific support for updates in a libsvn_wc common 
function that is used for more than just this update scenario. E.g. svn status 
-u).

The current code (well; before your patch) applies changes on svn:externals on 
update. By comparing the old and new versions of the svn:externals property it 
can add/remove/switch externals.

Your new code can just add externals, and if the directory is later reverted 
the externals will stay as a detached working copy. (And if a different 
svn:external value is set for a directory you will get some hard to resolve 
issues, that you would never get into with just the old code).


I think fixing issue ##2267 needs a good design instead of a quick and dirty 
fix in one of the fundamental editor helper functions. (E.g. does this break 
backwards compatibility of our APIs somewhere?)


Looking further in the implementation: Why does the code walk the children of 
an added noded, but not the children of the children?

Bert 

RE: svn commit: r946826 - in /subversion/branches/1.6.x-issue2267: ./ CHANGES subversion/libsvn_wc/adm_crawler.c

[Bert Huijben]

> -Original Message-
> From: s...@apache.org [mailto:s...@apache.org]
> Sent: vrijdag 21 mei 2010 1:44
> To: comm...@subversion.apache.org
> Subject: svn commit: r946826 - in /subversion/branches/1.6.x-issue2267: ./
> CHANGES subversion/libsvn_wc/adm_crawler.c
> 
> Author: stsp
> Date: Thu May 20 23:43:53 2010
> New Revision: 946826
> 
> URL: http://svn.apache.org/viewvc?rev=946826&view=rev
> Log:
> On the 1.6.x-issue2267 branch, fix issue #2267, "support uncommitted
> svn:externals properties".
> 
> This is technically a backport of r946661, but it had to be rewritten.
> So I've done a record-only merge of r946661 from ^/subversion/trunk
> to officially merry this commit to its husband on trunk.
> 
> * subversion/libsvn_wc/adm_crawler.c
>   (report_revisions_and_depths): If a locally added directory has
>an svn:externals property set on it, include the externals in
>traversal info. This makes our caller pull the externals into
>the locally added directory as part of the update.
> 
> Modified:
> subversion/branches/1.6.x-issue2267/   (props changed)
> subversion/branches/1.6.x-issue2267/CHANGES   (props changed)
> subversion/branches/1.6.x-
> issue2267/subversion/libsvn_wc/adm_crawler.c
> 
> Propchange: subversion/branches/1.6.x-issue2267/
> --
> --- svn:mergeinfo (original)
> +++ svn:mergeinfo Thu May 20 23:43:53 2010
> @@ -70,4 +70,4 @@
>  /subversion/branches/tc_url_rev:874351-874483
>  /subversion/branches/tree-conflicts:868291-873154
>  /subversion/branches/tree-conflicts-notify:873926-874008
> -
> /subversion/trunk:875965,875968,876004,876012,876017,876019,876022,8760
> 24,876041-876042,876048,876051,876055-
> 876056,876059,876083,876091,876097,876101,876109,876123-
> 876125,876129,876132,876138,876160,876167,876175,876180,876185,876205,8
> 76223-
> 876225,876230,876233,876245,876252,876256,876283,876287,876312,876326-
> 876327,876330,876366,876372,876374,876376,876383,876386,876442,876456-
> 876457,876462-876464,876467,876469,876480,876486,876495-876497,876516-
> 876518,876524,876526,876583,876601,876614,876628,876633,876641,876659,8
> 76687,876689,876705,876715,876726,876760,876763,876794,876804,876815-
> 876816,876821,876825,876837,876840-876841,876843,876849,876857-
> 876858,876862,876873,876890,876897,876905,876908,876925,876931,876934,8
> 76948-876949,876953,876987,876993,877011,877014,877016,877028-
> 877029,877038,877119,877127,877146,877157,877191,877195,877203,877211,8
> 77230,877234,877237,877243,877249,877259,877261,877304,877319,877407,87
> 7437,877441-877442,877453,877459,877472,877544,877553,87756
> 
> 5,877568,877573,877593,877595,877597,877601,877612,877665,877667,877681
> ,877692,877696,877701,877720,877730,877784,877793,877797,877809,877815,
> 877819,877821,877842,877848,877853,877867,877869,877873,877901,877909,8
> 77916,877931,877942,877953,877964,877968,877970,877981-
> 877982,878005,878013,878015,878020,878046,878053,878062,878074,878080,8
> 78089,878091,878093,878095,878127,878129,878131,878142,878173-
> 878176,878216,878240,878242,878255,878269,878272,878279,878296-
> 878297,878303,878321,878335,878338,878341,878343,878353,878364,878367-
> 878368,878385,878399,878423,878426,878462,878484,878491,878498,878532,8
> 78595,878646,878659,878673,878682-878683,878690-
> 878691,878693,878723,878760-
> 878761,878873,878875,878877,878879,878905,878915,878924-
> 878925,878946,878949,878955,878960,878970,878981,879001,879033,879056,8
> 79074,879076,879081-879082,879093,879105,879126,879148,879170,879198-
> 879199,879201,879271,879293,879357,879375-879376,879403,879631,879635-
> 879636,879688,879709-879711,879747,879954,
>  879961,880082,880095,880105,880162,880226,880274-
> 880275,880370,880450,880461,880474,880525-
> 880526,880552,881905,884842,886164,886197,888715,888979,889081,889840,8
> 91672,892050,892085,895514,895653,896522,896915,898963,899826,899828,90
> 0797,901752,902093,904301,904394,904594,905303,905326,906256,906305,906
> 587,908980-
> 908981,917640,918211,922516,923389,923391,926151,926167,927323,927328,9
> 31209,931211,931392,931568,932942,933299
> +/subversion/trunk:875965,875968,876004,876012,876017,876019,876022,876
> 024,876041-876042,876048,876051,876055-
> 876056,876059,876083,876091,876097,876101,876109,876123-
> 876125,876129,876132,876138,876160,876167,876175,876180,876185,876205,8
> 76223-
> 876225,876230,876233,876245,876252,876256,876283,876287,876312,876326-
> 876327,876330,876366,876372,876374,876376,876383,876386,876442,876456-
> 876457,876462-876464,876467,876469,876480,876486,876495-876497,876516-
> 876518,876524,876526,876583,876601,876614,876628,876633,876641,876659,8
> 76687,876689,876705,876715,876726,876760,876763,876794,876804,876815-
> 876816,876821,876825,876837,876840-876841,876843,876849,876857-
> 876858,876862,876873,876890,876897,876905,876908,876925,876931,876934,8
> 76948-876949,876953,876987,876993,877011,877014,877016,877028-
> 877029

RE: svn commit: r946661 - /subversion/trunk/subversion/libsvn_wc/adm_crawler.c

[Bert Huijben]

> -Original Message-
> From: Stefan Sperling [mailto:s...@elego.de]
> Sent: vrijdag 21 mei 2010 12:05
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r946661 -
> /subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> 
> On Fri, May 21, 2010 at 11:42:13AM +0200, Bert Huijben wrote:
> > > -Original Message-
> > > From: s...@apache.org [mailto:s...@apache.org]
> > > Sent: donderdag 20 mei 2010 17:11
> > > To: comm...@subversion.apache.org
> > > Subject: svn commit: r946661 -
> > > /subversion/trunk/subversion/libsvn_wc/adm_crawler.c
> > >
> > > Author: stsp
> > > Date: Thu May 20 15:11:07 2010
> > > New Revision: 946661
> > >
> > > URL: http://svn.apache.org/viewvc?rev=946661&view=rev
> > > Log:
> > > Fix issue #2267, "support uncommitted svn:externals properties".
> > >
> > > * subversion/libsvn_wc/adm_crawler.c
> > >   (read_traversal_info): Rename to ...
> > >   (read_externals_info): ... this. We've been using an external_func
> > >callback instead of a traversal info for some time.
> > >   (report_revisions_and_depths): Rename local variable CHILDREN to
> > >BASE_CHILDREN for clarity (the children come from the BASE tree).
> > >If the caller provided an EXTERNAL_FUNC callback, check locally
> > >added directories for svn:externals as well and pass them to the
> > >callback. The caller will then pull externals into the added
directory.
> >
> > Hi,
> >
> > I'm not sure if this really fixes this issue for the common use cases.
> 
> What are the common use cases you have in mind?
> 
> The one I have in mind is where you want to test externals you have
> configured on a locally added directory, before commit.
> I tried the following and could not get svn to misbehave:
> 
>   - reverting the locally added dir
>   - removing the svn:externals propery from the locally added dir
>   - removing an external from the svn:externals property
>   - adding a new external to the svn:externals property
>   - running svn status in all above cases
> 
> I could not get svn to behave differently when I tried the
> above on a directory I had already committed.
> 
> > (And it introduces libsvn_client specific support for updates in a
> > libsvn_wc common function that is used for more than just this update
> > scenario. E.g. svn status -u).
> >
> > The current code (well; before your patch) applies changes on
> > svn:externals on update. By comparing the old and new versions of the
> > svn:externals property it can add/remove/switch externals.
> 
> When does it do that? I cannot get trunk to physically remove an
> external which was removed from svn:external. Here's what I tried:
> 
> $ cd svn-sandbox/trunk
> $ ls
> alphabeta epsilon/ gamma/
> $ svn mkdir d
> A d
> $ svn ci -mm
> Adding d
> 
> Committed revision 3.
> $ svn ps svn:externals '^/trunk/gamma gamma' d
> property 'svn:externals' set on 'd'
> $ svn up
> 
> Fetching external item into 'd/gamma'
> Ad/gamma/delta
> Updated external to revision 3.
> 
> Updated to revision 3.
> $ svn ci
> Sendingd
> 
> Committed revision 4.
> $ svn ps svn:externals '^/trunk/epsilon epsilon' d
> property 'svn:externals' set on 'd'
> $ svn up
> 
> Fetching external item into 'd/epsilon'
> Ad/epsilon/zeta
> Updated external to revision 4.
> 
> Updated to revision 4.
> $ svn diff
> 
> Property changes on: d
> __
> _
> Modified: svn:externals
> ## -1 +1 ##
> -^/trunk/gamma gamma
> +^/trunk/epsilon epsilon
> $ svn ci -mm
> Sendingd
> 
> Committed revision 5.
> $ svn up
> 
> Fetching external item into 'd/epsilon'
> External at revision 5.
> 
> At revision 5.
> $ svn st
> ?   d/gamma
> X   d/epsilon
> 
> 
> The old external 'gamma' stays behind as a detached WC.
> 
> > Your new code can just add externals, and if the directory is later
> > reverted the externals will stay as a detached working copy. (And if a
> > different svn:external value is set for a directory you will get some
> > hard to resolve issues, that you would never get into with just the
> > old code).
> 
> The same happens with an already committed directory, see above.

See the relegate_external() test in externals_tests.py for some ideas on how
to

RE: [PATCH v3] speed up svn_txdelta_apply_instructions

[Bert Huijben]

> -Original Message-
> From: Stefan Fuhrmann [mailto:stefanfuhrm...@alice-dsl.de]
> Sent: vrijdag 21 mei 2010 0:48
> To: dev@subversion.apache.org
> Subject: [PATCH v3] speed up svn_txdelta_apply_instructions
> 
> Hi there,
> 
> this is an improved version of the patch posted here:
> 
>   http://svn.haxx.se/dev/archive-2010-05/0002.shtml
> 
> The improvements address the issues listed there:
> 
>   http://svn.haxx.se/dev/archive-2010-05/0216.shtml
> 
> -- Stefan^2.
> 
> 
> [[[
> svn_txdelta_apply_instructions is relatively slow for long
> instruction sequences copying small pieces of data. This
> seems to be particularly visible in non-packed FSFS
> repositories.
> 
> This patch extracts invariants out from the 'for' loop,
> optimizes overlapping copies as well as small data copy
> runtime.
> 
> * subversion/libsvn_delta/text_delta.c
>   (fast_memcpy, patterning_copy): new functions,
>   optimized for our specific workload
>   (svn_txdelta_apply_instructions): reduce loop overhead,
>   use fast_memcpy and patterning_copy
> 
> patch by stefanfuhrmann < at > alice-dsl.de
> ]]]

+/* Unlike memmove() or memcpy(), create repeating patterns when 
+ * source and target range overlap. Returns a pointer to the first
+ * byte after the copied target range.
+ */
+static APR_INLINE char*
+patterning_copy(char *target, const char *source, apr_size_t len)
+{
+  const char *end = source + len;
+
+  /* Copy in larger chunks if source and target don't overlap
+   * closer than the size of the chunks (or don't overlap at all).
+   * Use the natural machine word as chunk size
+   * (for some architectures size_t is even a bit larger).
+   */
+  if (end + sizeof(apr_size_t) <= target)
+for (; source + sizeof (apr_size_t) <= end; 
+   source += sizeof (apr_size_t), 
+   target += sizeof (apr_size_t))
+  *(apr_size_t*)(target) = *(apr_size_t*)(source);
+
+  /* Copy trailing bytes */
+  for (; source != end; source++)
+*(target++) = *source;
+
+  return target;
+}
+

patterning_copy() should check the alignment of source and destination or
the copies by using this blocksize can be much slower than the original
version that just used bytes. (On some architectures an unaligned operation
is completely handled in software from within an exception handler)

Bert

RE: [PATCH v3] speed up svn_txdelta_apply_instructions

[Bert Huijben]

> -Original Message-
> From: Stefan Sperling [mailto:s...@elego.de]
> Sent: vrijdag 21 mei 2010 14:51
> To: Julian Foad
> Cc: Stefan Fuhrmann; dev@subversion.apache.org
> Subject: Re: [PATCH v3] speed up svn_txdelta_apply_instructions
> 
> On Fri, May 21, 2010 at 12:54:08PM +0100, Julian Foad wrote:
> > On Fri, 2010-05-21 at 00:47 +0200, Stefan Fuhrmann wrote:
> > > -  /* Check that we produced the right amount of data.  */
> > > -  assert(tpos == window->tview_len);
> >
> > The original code looped through 'window->num_ops' operations, and
> > afterwards asserted that the amount of target data generated by them
> was
> > the expected amount.
> >
> > The new code loops until the expected amount of target data has been
> > generated by (some of) the operations.  I think, to preserve the
> > equivalent self-checking, it should then assert that exactly
> > 'window->num_ops' operations have been used:
> >
> >   assert(op == last_op);
> 
> Please use SVN_ERR_ASSERT_NO_RETURN() instead of plain assert().

You can freely use assert if you only want it to run in debug mode.
(assert() calls are automatically removed by the preprocessor in RELEASE
builds, so library users don't have an issue with these calls).

Bert