Re: svn commit: r986466 - /subversion/trunk/subversion/svnrdump/dump_editor.c

[Ramkumar Ramachandra]

Hi Bert,

Bert Huijben writes:
> copyfrom_path is probably relative from the repository or session root (not 
> sure which in your case). You are now appending the basename of 
> a/paths/basename and then joining the copyfrom_path after it. Shouldn't that 
> be the other way around?

Really silly mistake, and this is very apparent from the diff. Thanks
for catching; fixed in r986567 :)

-- Ram

Re: Proposal: Change repository's UUID over RA layer

[Ramkumar Ramachandra]

Hi Hyrum,

Hyrum K. Wright writes:
> On Sun, Aug 1, 2010 at 1:17 AM, Ramkumar Ramachandra  
> wrote:
> > Hi,
> >
> > I've found that I need the functionality to change a repository's UUID
> > over the RA layer in 'svnrdump load' (see my recently committed tests
> > to see why). I initially planned to put this off until functionality
> > until someone thinks I'm capable of handling this and gets me commit
> > access to libsvn_ra. However, Daniel asked me not to expect this and
> > start working on a branch instead after notifying the list- I've
> > therefore started a new ra-uuid branch for this (see r981164). Expect
> > to see some commits soon :)
> 
> It looks like this idea has been discussed, but probably won't go
> forward.  It's probably time to remove the branch.

Indeed. Thanks for the reminder. Branch removed in r986566.

-- Ram

Re: RFC: WCNG and Issue #2915: Merge tracking and missing subtrees due to non-svn removal

[Paul Burba]

On Thu, Aug 12, 2010 at 2:51 PM, Daniel Shahaf  wrote:
> Paul Burba wrote on Fri, Aug 06, 2010 at 10:30:50 -0400:
>> As described in issue #2915, in 1.6 when a merge target is a missing
>> subtree due to its removal by non-svn means, we try to record
>> mergeinfo such that the missing subtree doesn't have (or inherit)
>> mergeinfo describing the merge:
>>
>> (If you already have a vague idea of how this works you can skip to
>> 'You might suggest that it makes more sense' below for the RFC)
>>
>> ### A file is missing in our merge target
>>
>>   1.6.13-dev>svn st
>>   !       A_COPY\D\H\psi
>>
>> ### No initial mergeinfo
>>
>>   1.6.13-dev>svn pg svn:mergeinfo -vR
>>
>> ### Merge tries to apply change to missing file, but can't
>> ### and reports it as skipped
>>
>>   1.6.13-dev>svn merge ^^/A A_COPY -r2:4
>>   --- Merging r3 through r4 into 'A_COPY':
>>   U    A_COPY\D\G\rho
>>   Skipped missing target: 'A_COPY\D\H\psi'
>>   Summary of conflicts:
>>     Skipped paths: 1
>>
>> ### Merge target gets mergeinfo describing the merge
>> ### performed and the missing file gets empty "override"
>> ### mergeinfo so it doesn't inherit the target's mergeinfo
>>
>>   1.6.13-dev>svn st
>>    M      A_COPY
>>   M       A_COPY\D\G\rho
>>   !M      A_COPY\D\H\psi
>>
>>   1.6.13-dev>svn pg svn:mergeinfo -vR
>>   Properties on 'A_COPY\D\H\psi':
>>     svn:mergeinfo
>>
>>   Properties on 'A_COPY':
>>     svn:mergeinfo
>>       /A:3-4
>>
>> If the missing subtree was a directory we obviously can't set its
>> properties, so we treat this as a tree conflict:
>>
>>   1.6.13-dev>svn st
>>   !       A_COPY\D\H
>>
>>   1.6.13-dev>svn merge ^^/A A_COPY -r2:4
>>   --- Merging r3 through r4 into 'A_COPY':
>>   U    A_COPY\D\G\rho
>>      C A_COPY\D\H
>>   Summary of conflicts:
>>     Tree conflicts: 1
>>
>>   1.6.13-dev>svn st
>>    M      A_COPY
>>   M       A_COPY\D\G\rho
>>   !     C A_COPY\D\H
>>         >   local delete, incoming edit upon merge
>>
>> ~
>>
>> You might suggest that it makes more sense to simply throw an error
>> when a mergeinfo aware merge hits a missing subtree rather than
>> jumping through these hoops.  I wouldn't disagree, but an even better
>> solution was suggested to me recently by Bert: Restore the missing
>> subtrees.  With wcng's single DB this should be easy with
>> svn_wc_restore().
>>
>> Does anyone see a reason we should not restore missing subtrees
>> encountered during a merge?
>>
>> Assuming for a moment there is general agreement about the goodness of
>> this approach, which sounds better:
>>
>> A) Check for missing subtrees at the start of the merge and restore
>> them prior to any editor drives.  This would be easy enough to do in
>> libsvn_client/merge.c:get_mergeinfo_paths() which we call at the start
>> of a merges to collect information about subtrees "interesting" from a
>> merge-tracking perspective.  The drawback here is that we need to
>> detect all the missing subtrees and that means a new call to
>> svn_io_check_path/apr_stat for every path in the merge target.  Since
>> 99.99%* of merges don't involve missing subtrees, we are imposing a
>> needless performance penalty on most users.
>>
>
> Agreed: stat() on every file on, say, our trunk during a merge from a
> branch, is too expensive.
>
>> B) Restore the missing subtrees on-the-fly when the svn_delta_editor_t
>> callbacks (i.e. open_directory, open_file) actually encounter a
>> missing subtree.  This keeps the svn_io_check_path() calls to a
>> minimum.  The drawback is that missing subtrees untouched by the
>> editor are still missing after the merge.
>>
>
> *nod*
>
>> Any preference or suggestions for a superior solution are appreciated.
>>
>
> We could treat missing files as conflicts?  e.g., about the same as what
> we'd do if someone truncated the file to zero length.
>
> That way all information is present locally (so there will be no need to
> repeat the merge).

(Sorry for the tardy reply, I've been on vacation and wonderfully out
of the loop the last 4 days)

That is certainly an option, but how is it better than restoring the
missing file(s) and letting the merge complete?  WCNG with a single DB
enabled allows us to do that, it seems a *much* better solution that
raising what is almost certainly a spurious conflict?  No?  Or am I
missing something?

Paul

Re: svn commit: r986332 - in /subversion/trunk/subversion/libsvn_wc: wc-queries.sql wc_db.c

[Erik Huelsmann]

>>
>> Modified:
>>     subversion/trunk/subversion/libsvn_wc/wc-queries.sql
>>     subversion/trunk/subversion/libsvn_wc/wc_db.c
>
> Your log message doesn't describe any changes in wc_db.c

Prop-edited now. Thanks.

Bye,

Erik.

svn diff optimization to make blame faster?

[Johan Corveleyn]

Hi devs,

While "Looking to improve performance of svn annotate" [1], I found
that the current blame algorithm is mainly client-side bound, and that
most of its time is spent on "svn diff" (calls to svn_diff_file_diff_2
from add_file_blame in blame.c). Apart from avoiding to build
full-texts and diffing them altogether (which is subject of further
discussion in [1]), I'm wondering if optimization of "svn diff"
wouldn't also be an interesting way to improve the speed of blame.

So the main question is: is it worth it to spend time to analyze this
further and try to improve performance? Or has this already been
optimized in the past, or is it simply already as optimal as it can
get? I have no idea really, so if anyone can shed some light ...

Gut feeling tells me that there must be room for optimization, since
GNU diff seems a lot faster than svn diff for the same large file
(with one line changed) on my machine [1]. But maybe svn's diff
algorithm is purposefully different (better? more accurate? ...) than
GNU's, or there are specific things in the svn context so svn diff has
to do more work.

Any thoughts?

-- 
Johan

[1] http://svn.haxx.se/dev/archive-2010-08/0408.shtml

Re: svn commit: r986510 - in /subversion/trunk/subversion: bindings/javahl/native/SVNClient.cpp include/svn_client.h libsvn_client/delete.c libsvn_client/deprecated.c libsvn_client/externals.c libsvn_

[Hyrum K. Wright]

I raised some issues about this proposed change in
http://svn.haxx.se/dev/archive-2010-08/0431.shtml.  Could you please
address them?

Thanks,
-Hyrum

On Tue, Aug 17, 2010 at 5:25 PM,   wrote:
> Author: rhuijben
> Date: Tue Aug 17 22:25:09 2010
> New Revision: 986510
>
> URL: http://svn.apache.org/viewvc?rev=986510&view=rev
> Log:
> Following up on r957917, make it possible to switch of the ambient depth
> filter when using svn_client_status5, like it is possible with
> svn_client_update3().
>
> Currently we don't expose this information to the svn client, as using
> --set-depth on svn status would be illogical.
>
> * subversion/bindings/javahl/native/SVNClient.cpp
>  (SVNClient::status): Update caller.
>
> * subversion/include/svn_client.h
>  (svn_client_status5): Add flag.
>  (svn_client_status4): Update documentation.
>
> * subversion/libsvn_client/delete.c
>  (svn_client__can_delete): Update caller.
>
> * subversion/libsvn_client/deprecated.c
>  (svn_client_status4): Update caller.
>
> * subversion/libsvn_client/externals.c
>  (svn_client__do_external_status): Update caller.
>
> * subversion/libsvn_client/status.c
>  (svn_client_status5): Allow using the explicit depth for the ra call.
>
> * subversion/svn/status-cmd.c
>  (svn_cl__status): Update caller.
>
> Modified:
>    subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp
>    subversion/trunk/subversion/include/svn_client.h
>    subversion/trunk/subversion/libsvn_client/delete.c
>    subversion/trunk/subversion/libsvn_client/deprecated.c
>    subversion/trunk/subversion/libsvn_client/externals.c
>    subversion/trunk/subversion/libsvn_client/status.c
>    subversion/trunk/subversion/svn/status-cmd.c
>
> Modified: subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp?rev=986510&r1=986509&r2=986510&view=diff
> ==
> --- subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp 
> (original)
> +++ subversion/trunk/subversion/bindings/javahl/native/SVNClient.cpp Tue Aug 
> 17 22:25:09 2010
> @@ -183,7 +183,7 @@ SVNClient::status(const char *path, svn_
>     SVN_JNI_ERR(svn_client_status5(&youngest, ctx, checkedPath.c_str(),
>                                    &rev,
>                                    depth,
> -                                   getAll, onServer, noIgnore,
> +                                   getAll, onServer, noIgnore, FALSE,
>                                    ignoreExternals,
>                                    changelists.array(requestPool),
>                                    StatusCallback::callback, callback,
>
> Modified: subversion/trunk/subversion/include/svn_client.h
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_client.h?rev=986510&r1=986509&r2=986510&view=diff
> ==
> --- subversion/trunk/subversion/include/svn_client.h (original)
> +++ subversion/trunk/subversion/include/svn_client.h Tue Aug 17 22:25:09 2010
> @@ -2142,6 +2142,10 @@ typedef svn_error_t *(*svn_client_status
>  * definition, and with #svn_wc_notify_status_completed
>  * after each.
>  *
> + * If @a depth_as_sticky is set and @a depth is not
> + * #svn_depth_unknown, then the status is calculated as if depth_is_sticky
> + * was passed to an equivalent update command.
> + *
>  * @a changelists is an array of const char * changelist
>  * names, used as a restrictive filter on items whose statuses are
>  * reported; that is, don't report status about any item unless
> @@ -2162,6 +2166,7 @@ svn_client_status5(svn_revnum_t *result_
>                    svn_boolean_t update,
>                    svn_boolean_t no_ignore,
>                    svn_boolean_t ignore_externals,
> +                   svn_boolean_t depth_as_sticky,
>                    const apr_array_header_t *changelists,
>                    svn_client_status_func_t status_func,
>                    void *status_baton,
> @@ -2169,7 +2174,8 @@ svn_client_status5(svn_revnum_t *result_
>
>  /**
>  * Same as svn_client_status5(), but using #svn_wc_status_func3_t
> - * instead of #svn_wc_status_func4_t.
> + * instead of #svn_wc_status_func4_t and depth_as_sticky set to FALSE.
> + * (
>  *
>  * @since New in 1.6.
>  * @deprecated Provided for backward compatibility with the 1.6 API.
>
> Modified: subversion/trunk/subversion/libsvn_client/delete.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/delete.c?rev=986510&r1=986509&r2=986510&view=diff
> ==
> --- subversion/trunk/subversion/libsvn_client/delete.c (original)
> +++ subversion/trunk/subversion/libsvn_client/delete.c Tue Aug 17 22:25:09 
> 2010
> @@ -115,7 +115,7 @@ svn_client__can_delete(const char *path,
>      be deleted. */
>   r

Branch audit

[Hyrum K. Wright]

It's that time again!  In looking at the list of current branches
(excepting release and backport branches I've noticed several which
look to be abandoned or unmaintained.  Branches which have not been
touched in over a year include (along with author and date of last
revision, stolen from ViewVC):

  artem-soc-work/r863880 3 years djames  Really, 
actually, merge
r23802 and r23803 from trunk to the artem-soc-work branc…
  atomic-revprop/r984264 6 days  danielshTweak the 
behaviour of a
recently-added API to allow conveniently passing errors…
  bdb-fixes/ r858217 4 years brane   Working on issue 
#2449. Move
error_info refcount management where it belongs, t…
  capabilities-abstraction/  r869334 2 years kfogel  On the
capabilities-abstraction branch: Start unifying some of the
capabilities…
  issue-2699-dev/r863182 3 years dlr On the 
issue-2699-dev
branch: Improve locking error message. * subversion/libsv…
  issue-2897/r869309 2 years kameshj On the 
issue-2897 branch:
Improve documentation. * subversion/libsvn_client/me…
  issue-2897-take2/  r875276 19 months   kameshj On 
issue-2897-take2:
Merge from ^/trunk revisions r35038 through r35201.
  issue-3081/r869126 2 years kfogel  On the issue-3081 
branch: *
3081-repro/: New directory, with patches, repro scr…
  meta-data-versioning/  r858153 4 years pmarek  Change the 
"live"
properties only if they have really changed. Without this chan…
  perl-bindings-improvements/r867175 2 years jpeacock   
 Merge with
tr...@27098
  record_exact_merge_and_commit_revs/r868556 2 years 
kameshj On
the 'record_exact_merge_and_commit_revs' branch, merge 28395:28481
from /trun…
  scheme-bindings/   r869778 2 years
  scons-build-system/r862146 3 years danderson   Make 
the source
path creator safer. * build/builder.py (SvnBuild._make_src_…
  server-l10n/   r857339 4 years dlr * 
subversion/mod_dav_svn/lang.c
(sort_lang_pref): Swap return values to correc…
  status--filter/r867402 2 years epg *** Just 
stashing this away
until after 1.5 branches. Add status --filter, for …
  svnserve-ssl/  r859467 4 years mbk Make svnserve SSL 
config items
repo-conf-dir-relative, rather than absolute path…
  youngest-common-ancestor/  r867405 2 years cmpilato   
 Begin work on
a client-side youngest-common-ancestor API. NOTE: This code is…

Could folks take a look at these branches and justify their continued
existence?  It would also be nice to include plans to continue working
on them, or issues which keep them from being merged to trunk.

Thanks,
-Hyrum

RE: svn commit: r986466 - /subversion/trunk/subversion/svnrdump/dump_editor.c

[Bert Huijben]

> -Original Message-
> From: artag...@apache.org [mailto:artag...@apache.org]
> Sent: dinsdag 17 augustus 2010 12:49
> To: comm...@subversion.apache.org
> Subject: svn commit: r986466 -
> /subversion/trunk/subversion/svnrdump/dump_editor.c
> 
> Author: artagnon
> Date: Tue Aug 17 19:48:32 2010
> New Revision: 986466
> 
> URL: http://svn.apache.org/viewvc?rev=986466&view=rev
> Log:
> * subversion/svnrdump/dump_editor.c
>   (open_file): Simplify a block of code that pushes to an array and
>   then calls svn_path_compose; use svn_relpath_join instead.
> 
> Modified:
> subversion/trunk/subversion/svnrdump/dump_editor.c
> 
> Modified: subversion/trunk/subversion/svnrdump/dump_editor.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/dum
> p_editor.c?rev=986466&r1=986465&r2=986466&view=diff
> ==
> 
> --- subversion/trunk/subversion/svnrdump/dump_editor.c (original)
> +++ subversion/trunk/subversion/svnrdump/dump_editor.c Tue Aug 17
> 19:48:32 2010
> @@ -499,21 +499,16 @@ open_file(const char *path,
>struct dir_baton *pb = parent_baton;
>const char *copyfrom_path = NULL;
>svn_revnum_t copyfrom_rev = SVN_INVALID_REVNUM;
> -  apr_array_header_t *compose_path;
> 
>/* Some pending properties to dump? */
>SVN_ERR(dump_props(pb->eb, &(pb->eb->dump_props_pending), TRUE,
> pool));
> 
> -  compose_path = apr_array_make(pool, 2, sizeof(const char *));
> -
>/* If the parent directory has explicit copyfrom path and rev,
>   record the same for this one. */
>if (pb && ARE_VALID_COPY_ARGS(pb->copyfrom_path, pb-
> >copyfrom_rev))
>  {
> -  APR_ARRAY_PUSH(compose_path, const char *) = pb->copyfrom_path;
> -  APR_ARRAY_PUSH(compose_path, const char *) =
> -svn_relpath_basename(path, pool);
> -  copyfrom_path = svn_path_compose(compose_path, pool);
> +  copyfrom_path = svn_relpath_join(svn_relpath_basename(path, pool),
> +   pb->copyfrom_path, pool);
>copyfrom_rev = pb->copyfrom_rev;
>  }

copyfrom_path is probably relative from the repository or session root (not 
sure which in your case). You are now appending the basename of 
a/paths/basename and then joining the copyfrom_path after it. Shouldn't that be 
the other way around?

Bert 

Re: Proposal: Change repository's UUID over RA layer

[Hyrum K. Wright]

On Sun, Aug 1, 2010 at 1:17 AM, Ramkumar Ramachandra  wrote:
> Hi,
>
> I've found that I need the functionality to change a repository's UUID
> over the RA layer in 'svnrdump load' (see my recently committed tests
> to see why). I initially planned to put this off until functionality
> until someone thinks I'm capable of handling this and gets me commit
> access to libsvn_ra. However, Daniel asked me not to expect this and
> start working on a branch instead after notifying the list- I've
> therefore started a new ra-uuid branch for this (see r981164). Expect
> to see some commits soon :)

It looks like this idea has been discussed, but probably won't go
forward.  It's probably time to remove the branch.

-Hyrum

Re: svn commit: r985695 - /subversion/branches/performance/subversion/libsvn_subr/stream.c

[Stefan Fuhrmann]

 Hyrum K. Wright write:


This looks like it could be a candidate for 1.6.x.

-Hyrum


No, this is just a fix to some code I introduced myself in r985601.
Sorry, nothing to see here .. keep moving ..

-- Stefan^2.


On Sun, Aug 15, 2010 at 10:47 AM,  wrote:
/> Author: stefan2 /
/> Date: Sun Aug 15 15:47:38 2010 /
/> New Revision: 985695 /
/> /
/> URL: http://svn.apache.org/viewvc?rev=985695&view=rev 
 /

/> Log: /
/> Follow-up to r985601: fix a rare error condition that does not hurt 
functionality /
/> but performance (massively, so). stream_readline_chunky failed to 
detect EOL /
/> for longer lines until the end of the stream was encountered and 
would eventually /

/> fall back to byte-wise parsing the data. /
/> /
/> * subversion/libsvn_subr/stream.c /
/>  (stream_readline_chunky): fix EOL detection for long lines /
/> /
/> Modified: /
/>subversion/branches/performance/subversion/libsvn_subr/stream.c /
/> /
/> Modified: 
subversion/branches/performance/subversion/libsvn_subr/stream.c /
/> URL: 
http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/stream.c?rev=985695&r1=985694&r2=985695&view=diff 
 
/
/> 
== 
/
/> --- subversion/branches/performance/subversion/libsvn_subr/stream.c 
(original) /
/> +++ subversion/branches/performance/subversion/libsvn_subr/stream.c 
Sun Aug 15 15:47:38 2010 /

/> @@ -433,7 +433,7 @@ stream_readline_chunky(svn_stringbuf_t * /
/>   { /
/> /* Append the next chunk to the string read so far. /
/>  */ /
/> -svn_stringbuf_ensure (str, str->len + LINE_CHUNK_SIZE); /
/> +svn_stringbuf_ensure(str, str->len + LINE_CHUNK_SIZE); /
/> numbytes = LINE_CHUNK_SIZE; /
/> SVN_ERR(svn_stream_read(stream, str->data + str->len, 
&numbytes)); /

/> str->len += numbytes; /
/> @@ -456,7 +456,7 @@ stream_readline_chunky(svn_stringbuf_t * /
/>  * previous chunk because the EOL may span over the boundary /
/>  * between both chunks. /
/>  */ /
/> -eol_pos = strstr(str->data + str->len - (eol_len-1), eol); /
/> +eol_pos = strstr(str->data + str->len - numbytes - 
(eol_len-1), eol); /

/>   } /
/>   while (eol_pos == NULL); /
/> /

Re: svn commit: r985514 - stream_move_mark()

[Stefan Fuhrmann]

Stefan Sperling wrote:

On Mon, Aug 16, 2010 at 10:04:31AM +0100, Julian Foad wrote:
  

On Mon, 2010-08-16 at 09:55 +0100, Julian Foad wrote:


On Sat, 2010-08-14, stef...@apache.org wrote:
  

Log:
Extend the stream API by three functions:
svn_stream_move_mark() to move an existing mark by some delta


Hi Stefan.

Unfortunately this is not a logical extension to the stream API.
Subversion's svn_stream_t streams support arbitrary "filtering" or
"translation" tasks that require a mostly sequential processing of the
stream.  One of the ideas embodied by the "get a mark" and "seek to a
mark" paradigm is that the stream position is not expressible by a
simple number (such as a byte offset from the start), because the stream
may be performing translation whose state at a given position depends on
the preceding content of the stream.  Therefore "get a mark" does not
return a scalar number but instead returns a stream state object which
contains the translation state of the stream as well as the "mark" of
the underlying stream if there is one.

Seeking to an arbitrary new position requires telling the stream all of
its relevant internal state at that position.  "Move by +/- N bytes"
does not seem to be an operation that can be supported.
  

I overgeneralized my use-case here: I actually needed "move forward" only.
The concept of "skip N bytes", however, is perfectly in line with the 
general

stream semantics. Because this also fits my needs, I changed the API in
r986485 accordingly.

... supported by all streams that support mark & seek.  Of course some
streams could support it.

I don't like to see an API be complicated by lots of functions that may
or may not be supported, and functions to find out whether they are
supported.  I would ask you to look for another way to realize most of
the speed gain that you were hoping to get from this extension.


The impact of this change is pretty large as all FSFS structures except for
the actual delta info will be processed by parsers using stream_readline().
Reading that data byte-by-byte from the APR file is by far the most 
expensive

part of that parsing process. So, I would like to see that API extension
being accepted.

If it provides a real performance advantage, maybe we should just switch
some APIs which use streams to APR file handles where it makes sense.
Files can be seeked via byte-offsets.

The stream abstraction is nice, but it can get in the way.
Even mark/seek support as currently implemented is already beyond the
original idea of the stream abstraction. But we needed it for the patch code
which is using streams to provide an interface for reading text from patch
files in various ways. It's a nice abstraction for that purpose and should
remain, but maybe Stefan Fuhrmann has good reasons for not using the
stream abstraction elsewhere. 
  

The problem is that a lot of parser code would need to change at least its
function signatures to string buffers and offsets instead of streams. 
I'm not

even sure that all streams are based on strings; some may be parsed on
APR file as well and use the same parser code (e.g. read hash from stream).

IOW, the least risky alternative approach would probably be the introduction
of a "kind-of-stream" concept that supports the extra functionality.

-- Stefan^2.

Re: svn commit: r985487 - in /subversion/branches/performance/subversion: include/svn_io.h libsvn_subr/io.c libsvn_subr/stream.c

[Stefan Fuhrmann]

Daniel Shahaf wrote:

stef...@apache.org wrote on Sat, Aug 14, 2010 at 13:20:22 -:
  

Author: stefan2
Date: Sat Aug 14 13:20:22 2010
New Revision: 985487

URL: http://svn.apache.org/viewvc?rev=985487&view=rev
Log:
APR file I/O is a high frequency operation as the respective streams directly 
map their requests
onto the file layer. Thus, introduce svn_io_file_putc() as symmetrical 
counterpart to *_getc()
and use both for single char read requests because their APR implementation 
tends to involve
far less overhead.

Also, introduce and use svn_io_file_read_full2 that optionally doesn't create 
an error object
upon EOF that may be discarded by the caller anyways. This actually translates 
into significant
runtime savings because constructing the svn_error_t for file errors involves 
expensive locale
dependent functions.

* subversion/include/svn_io.h
  (svn_io_file_putc, svn_io_file_read_full2): declare new API functions
* subversion/libsvn_subr/svn_io.c
  (svn_io_file_putc, svn_io_file_read_full2): implement new API functions
* subversion/libsvn_subr/stream.c
  (read_handler_apr, write_handler_apr): use the I/O function best suitable for 
the request

Modified:
subversion/branches/performance/subversion/include/svn_io.h
subversion/branches/performance/subversion/libsvn_subr/io.c
subversion/branches/performance/subversion/libsvn_subr/stream.c

Modified: subversion/branches/performance/subversion/include/svn_io.h
URL: 
http://svn.apache.org/viewvc/subversion/branches/performance/subversion/include/svn_io.h?rev=985487&r1=985486&r2=985487&view=diff
==
--- subversion/branches/performance/subversion/include/svn_io.h (original)
+++ subversion/branches/performance/subversion/include/svn_io.h Sat Aug 14 
13:20:22 2010
@@ -1745,6 +1745,15 @@ svn_io_file_getc(char *ch,
  apr_pool_t *pool);
 
 
+/** Wrapper for apr_file_putc(). 
+  * @since New in 1.7

+  */
+svn_error_t *
+svn_io_file_putc(char ch,
+ apr_file_t *file,
+ apr_pool_t *pool);
+
+
 /** Wrapper for apr_file_info_get(). */
 svn_error_t *
 svn_io_file_info_get(apr_finfo_t *finfo,
@@ -1770,6 +1779,20 @@ svn_io_file_read_full(apr_file_t *file,
   apr_pool_t *pool);
 
 
+/** Wrapper for apr_file_read_full(). 
+ * If eof_is_ok is set, no svn_error_t error object

+ * will be created upon EOF.
+ * @since New in 1.7
+ */
+svn_error_t *
+svn_io_file_read_full2(apr_file_t *file,
+   void *buf,
+   apr_size_t nbytes,
+   apr_size_t *bytes_read,
+   svn_boolean_t eof_is_ok,
+   apr_pool_t *pool);
+
+



Why didn't you deprecate svn_io_file_read_full() in the same commit?
  

By the time I wrote that code almost 3 months ago, I tried to minimize
the impact (code churn) on the code base.

Usually we do it as follows:

+ the declaration of svn_io_file_read_full2() is *above* that
  of svn_io_file_read_full (not critical)
+ mark the old function SVN_DEPRECATED (preprocessor symbol) and
  doxygen @deprecated
+ change the doc string of the old function to be a diff against the new
  function's docs
+ in the appropriate *.c file, upgrade the definition in-place
+ re-define the old function in lib_$foo/deprecated.c as a wrapper around the
  new function
  

Took me a number of commits but it should be o.k. by r986492.

Why you didn't make svn_io_file_read_full() a deprecated wrapper around
svn_io_file_read_full2()?
  

See above ;)

-- Stefan^2.

Re: svn_client_status5() and depth

[Hyrum K. Wright]

On Tue, Aug 17, 2010 at 3:45 PM, C. Michael Pilato  wrote:
> On 08/17/2010 01:42 PM, Stefan Küng wrote:
>> Maybe I don't understand that change:
>> --depth specifies a depth to use for the command. If I want the command
>> to use the depth of the working copy, I specify an unknown depth or none
>> at all. But if I specify a depth, I would assume the command to respect
>> that depth and return the info with that depth.
>> So why should the -u flag not use the specified depth?
>
> --depth is a filtering option -- it can only reduce the scope of an
> operation, it can not expand it.
>
> Bert sez he'll make the behavior optional in the API call so you can make
> use of it.  We won't expose it at all in the 'svn' command-line client, but
> TortoiseSVN can get its behavior back.  Sound good?


Yay for another API flag!


I'm starting to wonder if the mission (at the API level) of 'st -u'
and vanilla 'st' has diverged enough that we may want to consider an
API named svn_client_remote_status() or something.  It seems a bit
silly to keep extending APIs using various boolean flags[1], in a
manner which eventually leads to both consumer and programmer
confusion.  This eventually leads to entry points whose behavior
wildly varies based upon the permutations of the set of flags.
Perhaps status5 (with its 13 parameters) has reached this point?

-Hyrum

[1] I will readily admit to being as guilty of this as anybody else,
but the first step in repentance is recognition, and the next is, uh,
proselyting a new-found vision of The Way things should be.

Re: svn_client_status5() and depth

[Stefan Küng]

On 17.08.2010 22:45, C. Michael Pilato wrote:

On 08/17/2010 01:42 PM, Stefan Küng wrote:

Maybe I don't understand that change:
--depth specifies a depth to use for the command. If I want the command
to use the depth of the working copy, I specify an unknown depth or none
at all. But if I specify a depth, I would assume the command to respect
that depth and return the info with that depth.
So why should the -u flag not use the specified depth?


--depth is a filtering option -- it can only reduce the scope of an
operation, it can not expand it.

Bert sez he'll make the behavior optional in the API call so you can make
use of it.  We won't expose it at all in the 'svn' command-line client, but
TortoiseSVN can get its behavior back.  Sound good?



Sounds good, yes.

I think I got confused a little bit: the API for svn_client_status() 
only has a depth param. and svn_client_update has a depth param and a 
flag 'depth_is_sticky', so there's no two depth params as in the CL - 
one for filtering and one for setting.


Would the svn_client_status3 API get another depth param for this? Or a 
flag to allow a depth deeper than the depth of the WC?


Stefan

--
   ___
  oo  // \\  "De Chelonian Mobile"
 (_,\/ \_/ \ TortoiseSVN
   \ \_/_\_/>The coolest Interface to (Sub)Version Control
   /_/   \_\ http://tortoisesvn.net

Re: svn_client_status5() and depth

[Stefan Küng]

On 17.08.2010 22:29, Bert Huijben wrote:


But 'svn status' with a --set-depth would mean that this call could
change the WC?
Until now, 'svn st' didn't change the WC at all, i.e., it was a
read-only operation. So is such a change really wanted or necessary?


No it doesn't change the WC, but it sends different information to the
network.
In one case the depth stored in the working copy is used (to filter the
information you miss in your example) and in the other case the information
isn't filtered.

svn status -u was designed to show what an equivalent 'svn up' would do and
in this case it didn't do that. Instead it just showed everything below,
without looking at the ambient depth.


Hmm, you're right. An 'svn up --depth infinity' also doesn't extend a 
sparse checkout. But 'svn up --set-depth infinity' does.


So, '--depth ARG' only does something if ARG is less or equal the depth 
of the working copy. If it is more (a deeper depth) then it just does 
nothing, i.e., uses the depth of the working copy.


I think for read-only operations, --depth ARG should work with a deeper 
depth too. For write operations like update, a deeper depth would 
automatically imply '--set-depth' (depth_is_sticky in the 
svn_client_update3 API), so for those the --set-depth is necessary to 
use the deeper depth.


Stefan

--
   ___
  oo  // \\  "De Chelonian Mobile"
 (_,\/ \_/ \ TortoiseSVN
   \ \_/_\_/>The coolest Interface to (Sub)Version Control
   /_/   \_\ http://tortoisesvn.net

Re: svn_client_status5() and depth

[C. Michael Pilato]

On 08/17/2010 01:42 PM, Stefan Küng wrote:
> Maybe I don't understand that change:
> --depth specifies a depth to use for the command. If I want the command
> to use the depth of the working copy, I specify an unknown depth or none
> at all. But if I specify a depth, I would assume the command to respect
> that depth and return the info with that depth.
> So why should the -u flag not use the specified depth?

--depth is a filtering option -- it can only reduce the scope of an
operation, it can not expand it.

Bert sez he'll make the behavior optional in the API call so you can make
use of it.  We won't expose it at all in the 'svn' command-line client, but
TortoiseSVN can get its behavior back.  Sound good?

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand



signature.asc
Description: OpenPGP digital signature

Re: svn_client_status5() and depth

[Stefan Küng]

On 17.08.2010 22:32, C. Michael Pilato wrote:

On 08/17/2010 12:44 PM, Stefan Küng wrote:

Like I said in r957917, I think we should fix this in a different way.

The difference is between:
What would you get with
* svn up --depth infinity
and
* svn up --set-depth infinity

svn status --depth infinity used to show the last variant, but shows the
first variant now.

I think we should add an option to choose between those two variants. (By
enabling --set-depth on 'svn status' and a similar change to
libsvn_client)


But 'svn status' with a --set-depth would mean that this call could
change the WC?
Until now, 'svn st' didn't change the WC at all, i.e., it was a
read-only operation. So is such a change really wanted or necessary?


Agreed.  'svn st' is a read-only operation, period.  But I suspect that Bert
is saying that the --set-depth option, when applied to 'svn status -u',
would be merely a way to describe the "mode" of the -u?


Isn't that what --depth is for?
As I understand it, --depth is to specify the depth you want, and 
--set-depth is the depth you want the target to set to (i.e., keep that 
depth after the operation).



If that's the case, personally I think it would be a horrendous UI decision.
  Any option that has "set" as its action-word sounds like something's about
to be changed, which is not what's really going on here.


That's how I understand it: --set-depth sets the depth. But of course 
it's likely that I'm wrong here.



But that said, I believe Bert's change in r957917 was a good one -- a
correct one -- perfectly in line with the interpretation of the -u option to
'svn st' altogether.  So I'm interested, Stefan, in understanding why
TortoiseSVN wants the prior behavior.  What's TortoiseSVN trying to tell its
users with this extra data?


I use this in the "check for modifications" dialog which shows the 
status of the whole working copy in a flat view. There's a 'check 
repository' button which uses the '-u' flag.
Now, if users have a sparse checkout, they can use the 'check 
repository' button and get all the files that are not in their sparse 
checkout but in the repository. Then users can right-click on those 
files and update those - which is used to populate a sparse checkout.
And also, if new folders are added users can 'extend' their sparse 
checkout that way, so those new folders have to show up with an 'svn st 
-u -v --depth infinity' so users can actually see that there's something 
new in the repository.


Maybe I don't understand that change:
--depth specifies a depth to use for the command. If I want the command 
to use the depth of the working copy, I specify an unknown depth or none 
at all. But if I specify a depth, I would assume the command to respect 
that depth and return the info with that depth.

So why should the -u flag not use the specified depth?

Stefan

--
   ___
  oo  // \\  "De Chelonian Mobile"
 (_,\/ \_/ \ TortoiseSVN
   \ \_/_\_/>The coolest Interface to (Sub)Version Control
   /_/   \_\ http://tortoisesvn.net

Re: svn_client_status5() and depth

[C. Michael Pilato]

On 08/17/2010 12:44 PM, Stefan Küng wrote:
>> Like I said in r957917, I think we should fix this in a different way.
>>
>> The difference is between:
>> What would you get with
>> * svn up --depth infinity
>> and
>> * svn up --set-depth infinity
>>
>> svn status --depth infinity used to show the last variant, but shows the
>> first variant now.
>>
>> I think we should add an option to choose between those two variants. (By
>> enabling --set-depth on 'svn status' and a similar change to
>> libsvn_client)
> 
> But 'svn status' with a --set-depth would mean that this call could
> change the WC?
> Until now, 'svn st' didn't change the WC at all, i.e., it was a
> read-only operation. So is such a change really wanted or necessary?

Agreed.  'svn st' is a read-only operation, period.  But I suspect that Bert
is saying that the --set-depth option, when applied to 'svn status -u',
would be merely a way to describe the "mode" of the -u?

If that's the case, personally I think it would be a horrendous UI decision.
 Any option that has "set" as its action-word sounds like something's about
to be changed, which is not what's really going on here.

But that said, I believe Bert's change in r957917 was a good one -- a
correct one -- perfectly in line with the interpretation of the -u option to
'svn st' altogether.  So I'm interested, Stefan, in understanding why
TortoiseSVN wants the prior behavior.  What's TortoiseSVN trying to tell its
users with this extra data?

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand



signature.asc
Description: OpenPGP digital signature

RE: svn_client_status5() and depth

[Bert Huijben]

> -Original Message-
> From: Stefan Küng [mailto:tortoise...@gmail.com]
> Sent: dinsdag 17 augustus 2010 12:45
> To: Bert Huijben
> Cc: 'Subversion Development'
> Subject: Re: svn_client_status5() and depth
> 
> On 17.08.2010 21:37, Bert Huijben wrote:
> >
> >
> >> -Original Message-
> >> From: Stefan Küng [mailto:tortoise...@gmail.com]
> >> Sent: dinsdag 17 augustus 2010 12:28
> >> To: Subversion Development
> >> Subject: svn_client_status5() and depth
> >>
> >> Hi,
> >>
> >> * check out a folder with depth svn_depth_empty (svn co
> >> file:///d:/testrepo/trunk testwc)
> >> * cd into that folder (cd testwc)
> >> * run 'svn st -u -v --depth infinity'
> >>
> >> In 1.6.x, I get the status for all the files under that folder that
> >> exist in the repository.
> >>
> >> With the latest trunk (r986453), I only get
> >>   32  32   user  .
> >> Status against revision:32
> >>
> >> All the files in the repository are not reported back.
> >>
> >> I'm relying on the behavior of the 1.6.x version in TSVN. If this
change
> >> was by design, why was this changed? I couldn't find anything in recent
> >> discussions about this. But that could easily be because of the
> >> not-very-good search feature of the mailing list archives.
> >>
> >> Or is this a bug?
> >
> > Hmm.. I think I know why you see this bug and I was responsible for
> > introducing it:
> >
> > Like I said in r957917, I think we should fix this in a different way.
> >
> > The difference is between:
> > What would you get with
> > * svn up --depth infinity
> > and
> > * svn up --set-depth infinity
> >
> > svn status --depth infinity used to show the last variant, but shows the
> > first variant now.
> >
> > I think we should add an option to choose between those two variants.
(By
> > enabling --set-depth on 'svn status' and a similar change to
libsvn_client)
> 
> But 'svn status' with a --set-depth would mean that this call could
> change the WC?
> Until now, 'svn st' didn't change the WC at all, i.e., it was a
> read-only operation. So is such a change really wanted or necessary?

No it doesn't change the WC, but it sends different information to the
network. 
In one case the depth stored in the working copy is used (to filter the
information you miss in your example) and in the other case the information
isn't filtered.

svn status -u was designed to show what an equivalent 'svn up' would do and
in this case it didn't do that. Instead it just showed everything below,
without looking at the ambient depth.

Bert

Re: svn_client_status5() and depth

[Stefan Küng]

On 17.08.2010 21:37, Bert Huijben wrote:




-Original Message-
From: Stefan Küng [mailto:tortoise...@gmail.com]
Sent: dinsdag 17 augustus 2010 12:28
To: Subversion Development
Subject: svn_client_status5() and depth

Hi,

* check out a folder with depth svn_depth_empty (svn co
file:///d:/testrepo/trunk testwc)
* cd into that folder (cd testwc)
* run 'svn st -u -v --depth infinity'

In 1.6.x, I get the status for all the files under that folder that
exist in the repository.

With the latest trunk (r986453), I only get
  32  32   user  .
Status against revision:32

All the files in the repository are not reported back.

I'm relying on the behavior of the 1.6.x version in TSVN. If this change
was by design, why was this changed? I couldn't find anything in recent
discussions about this. But that could easily be because of the
not-very-good search feature of the mailing list archives.

Or is this a bug?


Hmm.. I think I know why you see this bug and I was responsible for
introducing it:

Like I said in r957917, I think we should fix this in a different way.

The difference is between:
What would you get with
* svn up --depth infinity
and
* svn up --set-depth infinity

svn status --depth infinity used to show the last variant, but shows the
first variant now.

I think we should add an option to choose between those two variants. (By
enabling --set-depth on 'svn status' and a similar change to libsvn_client)


But 'svn status' with a --set-depth would mean that this call could 
change the WC?
Until now, 'svn st' didn't change the WC at all, i.e., it was a 
read-only operation. So is such a change really wanted or necessary?


Stefan

--
   ___
  oo  // \\  "De Chelonian Mobile"
 (_,\/ \_/ \ TortoiseSVN
   \ \_/_\_/>The coolest Interface to (Sub)Version Control
   /_/   \_\ http://tortoisesvn.net

RE: svn_client_status5() and depth

[Bert Huijben]

> -Original Message-
> From: Stefan Küng [mailto:tortoise...@gmail.com]
> Sent: dinsdag 17 augustus 2010 12:28
> To: Subversion Development
> Subject: svn_client_status5() and depth
> 
> Hi,
> 
> * check out a folder with depth svn_depth_empty (svn co
> file:///d:/testrepo/trunk testwc)
> * cd into that folder (cd testwc)
> * run 'svn st -u -v --depth infinity'
> 
> In 1.6.x, I get the status for all the files under that folder that
> exist in the repository.
> 
> With the latest trunk (r986453), I only get
>  32  32   user  .
> Status against revision:32
> 
> All the files in the repository are not reported back.
> 
> I'm relying on the behavior of the 1.6.x version in TSVN. If this change
> was by design, why was this changed? I couldn't find anything in recent
> discussions about this. But that could easily be because of the
> not-very-good search feature of the mailing list archives.
> 
> Or is this a bug?

Hmm.. I think I know why you see this bug and I was responsible for
introducing it:

Like I said in r957917, I think we should fix this in a different way.

The difference is between:
What would you get with
* svn up --depth infinity
and
* svn up --set-depth infinity

svn status --depth infinity used to show the last variant, but shows the
first variant now.

I think we should add an option to choose between those two variants. (By
enabling --set-depth on 'svn status' and a similar change to libsvn_client) 

Bert 

RE: svn_client_status5() and depth

[Bert Huijben]

> -Original Message-
> From: Stefan Küng [mailto:tortoise...@gmail.com]
> Sent: dinsdag 17 augustus 2010 12:28
> To: Subversion Development
> Subject: svn_client_status5() and depth
> 
> Hi,
> 
> * check out a folder with depth svn_depth_empty (svn co
> file:///d:/testrepo/trunk testwc)
> * cd into that folder (cd testwc)
> * run 'svn st -u -v --depth infinity'
> 
> In 1.6.x, I get the status for all the files under that folder that
> exist in the repository.
> 
> With the latest trunk (r986453), I only get
>  32  32   user  .
> Status against revision:32
> 
> All the files in the repository are not reported back.
> 
> I'm relying on the behavior of the 1.6.x version in TSVN. If this change
> was by design, why was this changed? I couldn't find anything in recent
> discussions about this. But that could easily be because of the
> not-very-good search feature of the mailing list archives.
> 
> Or is this a bug?

This looks like a bug to me. It should show all nodes with -v.

Bert 

svn_client_status5() and depth

[Stefan Küng]

Hi,

* check out a folder with depth svn_depth_empty (svn co 
file:///d:/testrepo/trunk testwc)

* cd into that folder (cd testwc)
* run 'svn st -u -v --depth infinity'

In 1.6.x, I get the status for all the files under that folder that 
exist in the repository.


With the latest trunk (r986453), I only get
32  32   user  .
Status against revision:32

All the files in the repository are not reported back.

I'm relying on the behavior of the 1.6.x version in TSVN. If this change 
was by design, why was this changed? I couldn't find anything in recent 
discussions about this. But that could easily be because of the 
not-very-good search feature of the mailing list archives.


Or is this a bug?

Stefan

--
   ___
  oo  // \\  "De Chelonian Mobile"
 (_,\/ \_/ \ TortoiseSVN
   \ \_/_\_/>The coolest Interface to (Sub)Version Control
   /_/   \_\ http://tortoisesvn.net

Re: svn commit: r106 - trunk/www: . cn-project-pages/snippets

[Jack Repenning]

On Aug 17, 2010, at 8:28 AM, C. Michael Pilato wrote:

> I've handled this reversion and reworked the text there to make the point
> that I believe Jack (in his role as a Tigris administrator) was mostly
> trying to make:  "Don't look for new stuff on this, our old site, and then
> complain to the Tigris admins when you can't find it."
> 
> 
> On 08/17/2010 12:08 AM, Daniel Shahaf wrote:
>> -1 on this change: it adds a link to openCollabNet to our tigris HOMEPAGE,
>> without any prior discussion.
>> 
>> jrepenning_cn: please revert this asap, and avoid making changes to our
>> Website in this manner.



Yes. Thanks for cleaning up my mess, Mike, and sorry for botching the protocol.

I guess I'm getting a little obsessive about this problem: we're still seeing 
20-40 thousand downloads per week of the stale Windows binaries in the 
subversion.tigris.org "Documents and files" area, installers no newer than 
1.6.6, and often a lot older. For instance, in the last week the top 10 
downloads from this area are:

| 4920 | Setup-Subversion-1.6.6.msi | 
| 3168 | Setup-Subversion-1.6.0.msi | 
| 3039 | svn-win32-1.6.6.zip| 
| 2600 | svn-win32-1.5.6.zip| 
| 2185 | svn-1.4.0-setup.exe| 
| 2107 | svn-1.4.5-setup.exe| 
| 1813 | svn-1.4.6-setup.exe| 
| 1767 | Setup-Subversion-1.6.5.msi | 
| 1056 | svn-win32-1.4.5.zip| 
| 1016 | svn-win32-1.4.3.zip| 

"svn-1.4.0-setup.exe"? I'm sorry ... "svn-1.4.0-setup.exe"? Two thousand people 
really need svn-1.4.0-setup.exe? It's particularly interesting that the 
download numbers spike every time there's a new Subversion release--as, of 
course, they should, except that I'm still talking about downloads of older 
releases. I'm just not buying it. I think these people are confused, and I'm 
trying to figure out how to help them find something more up-to-date.



-==-
Jack Repenning
Chief Technology Officer
CollabNet, Inc.
8000 Marina Boulevard, Suite 600
Brisbane, California 94005
office: +1 650.228.2562
twitter: http://twitter.com/jrep

Re: Webpage showing 1.7 status?

[C. Michael Pilato]

On 08/17/2010 09:59 AM, Hyrum K. Wright wrote:
> Howdy all.
> 
> All of the wandiscians are having a face-to-face this week, and one of
> the ideas floated was a public-facing page on our website listing the
> items left to complete before the 1.7 release.  In the past, we've had
> a STATUS-1.x in trunk, which has been useful for tracking the stages
> toward the release, and helping people who want to contribute know
> what needs to be done.  By putting that document on the web, we can
> also use it to communicate to the people who are asking about the
> status of 1.7.
> 
> We (Julian, Philip, Erik, and I) can seed this document with the
> results of our discussions this week, and with the release plan I
> posted a few weeks ago.
> 
> Thoughts?

I'm no fan of spawning tons of little webpages whose meaningful lifespan is
relatively short.  This is Subversion, so you can point folks directly into
the version control repository for stuff like that.  However, if you can
make a case for a webpage that can have a much longer lifespan and serve us
far into the future, +1 from me.  For example, I can see us maintaining a
subversion.apache.org/release-status.html page which serves multiple purposes:

   "Status of Current Release"

   ### Describe the status of the next .0 release we're working toward.
   ### This is the stuff you guys are suggesting.

   "Subversion Release Maintenance Policy"

   ### Explain our general policy of actively maintaining only one existing
   ### release line save for extremely critical security bug fixes that we
   ### might backport to even older release streams.

   "Status of Prior Releases"

   ### Note the status of each of our pre-1.7 release streams (1.6.x is open
   ### for maintenance, with a 1.6.13 release due on ?/?/2010; all others
   ### are closed to changes excepting severe security bug fixes.)

The one downside I see to any of this is that history has shown the
Subversion developers to be pretty slack about updating web pages.  I guess
we just don't tend to think most of the time in terms of "How can I better
help Joe Suitwearer understand how and to what degree the coding work I'm
doing is making an impact towards are next release?"

-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand



signature.asc
Description: OpenPGP digital signature

Webpage showing 1.7 status?

[Hyrum K. Wright]

Howdy all.

All of the wandiscians are having a face-to-face this week, and one of
the ideas floated was a public-facing page on our website listing the
items left to complete before the 1.7 release.  In the past, we've had
a STATUS-1.x in trunk, which has been useful for tracking the stages
toward the release, and helping people who want to contribute know
what needs to be done.  By putting that document on the web, we can
also use it to communicate to the people who are asking about the
status of 1.7.

We (Julian, Philip, Erik, and I) can seed this document with the
results of our discussions this week, and with the release plan I
posted a few weeks ago.

Thoughts?

-Hyrum

Re: Returning revprops through commit info

[Hyrum K. Wright]

On Mon, Aug 16, 2010 at 5:24 AM, Julian Foad  wrote:
> On Thu, 2010-08-12, Hyrum K. Wright wrote:
>> Recently, we've updated the various client APIs which do commits to
>> return commit info back through a callback[1], since they may be
>> extended to perform multiple commits (see issue #1199 for instance).
>> In doing so, I've had the opportunity to take a look at the
>> svn_commit_info_t struct.  It's pretty simplistic, but includes fields
>> for the author and date.
>>
>> In 1.6 (I believe) we changed the various log and commit APIs to use a
>> hash of arbitrary revision properties, rather than a hard coded list.
>> I wonder if it's worth it to do so in the commit_info struct.  We'd
>> still keep the existing fields for compat, but we would also add a
>> hash of revision properties, for consistency with the other APIs, and
>> for greater future extensibility.
>>
>> Thoughts?
>
> +1.

I'll go ahead and start working on this.  It may entail extending the
various RA protocols, but I hope to be able to do this on trunk
instead of a branch.  I know it isn't on the 1.7 blocking list, and
may seem like superflous code churn, but for completeness and
symmetry's sake, it just seems Right.  (and helps placate my OCD
:P )

>> [1] The callback was added as a member of the client context, instead
>> of a per-API func/baton set.  This choice was somewhat arbitrary, and
>> conversations with Mark regarding the same in the JavaHL wrappers have
>> made me wonder if we should go with the explicit func/baton args,
>> rather than using the client ctx.  Anybody have any strong feelings
>> about this issue?

Any thoughts on this issue?

-Hyrum

Re: svn commit: r985695 - /subversion/branches/performance/subversion/libsvn_subr/stream.c

[Hyrum K. Wright]

This looks like it could be a candidate for 1.6.x.

-Hyrum

On Sun, Aug 15, 2010 at 10:47 AM,   wrote:
> Author: stefan2
> Date: Sun Aug 15 15:47:38 2010
> New Revision: 985695
>
> URL: http://svn.apache.org/viewvc?rev=985695&view=rev
> Log:
> Follow-up to r985601: fix a rare error condition that does not hurt 
> functionality
> but performance (massively, so). stream_readline_chunky failed to detect EOL
> for longer lines until the end of the stream was encountered and would 
> eventually
> fall back to byte-wise parsing the data.
>
> * subversion/libsvn_subr/stream.c
>  (stream_readline_chunky): fix EOL detection for long lines
>
> Modified:
>    subversion/branches/performance/subversion/libsvn_subr/stream.c
>
> Modified: subversion/branches/performance/subversion/libsvn_subr/stream.c
> URL: 
> http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/stream.c?rev=985695&r1=985694&r2=985695&view=diff
> ==
> --- subversion/branches/performance/subversion/libsvn_subr/stream.c (original)
> +++ subversion/branches/performance/subversion/libsvn_subr/stream.c Sun Aug 
> 15 15:47:38 2010
> @@ -433,7 +433,7 @@ stream_readline_chunky(svn_stringbuf_t *
>       {
>         /* Append the next chunk to the string read so far.
>          */
> -        svn_stringbuf_ensure (str, str->len + LINE_CHUNK_SIZE);
> +        svn_stringbuf_ensure(str, str->len + LINE_CHUNK_SIZE);
>         numbytes = LINE_CHUNK_SIZE;
>         SVN_ERR(svn_stream_read(stream, str->data + str->len, &numbytes));
>         str->len += numbytes;
> @@ -456,7 +456,7 @@ stream_readline_chunky(svn_stringbuf_t *
>          * previous chunk because the EOL may span over the boundary
>          * between both chunks.
>          */
> -        eol_pos = strstr(str->data + str->len - (eol_len-1), eol);
> +        eol_pos = strstr(str->data + str->len - numbytes - (eol_len-1), eol);
>       }
>       while (eol_pos == NULL);
>
>
>
>

RE: svnrdump: The BIG update

[Bert Huijben]

> -Original Message-
> From: Ramkumar Ramachandra [mailto:artag...@gmail.com]
> Sent: dinsdag 17 augustus 2010 9:09
> To: Daniel Shahaf
> Cc: Subversion-dev Mailing List
> Subject: Re: svnrdump: The BIG update
> 
> Hi Daniel,
> 
> Daniel Shahaf writes:
> > Ramkumar Ramachandra wrote on Thu, Aug 12, 2010 at 12:17:34 +0530:
> > > > > The dump functionality is also complete- thanks to Stefan's review
> and
> > > > > MANY others for cleaning it up. It's however hit a brick wall now
> > > > > because of missing headers in the RA layer. Until I (or someone
else)
> > > > > figures out how to fix the RA layer, we can't do better than the
XFail
> > > > > copy-and-modify test I've committed.
> > > >
> > > > Part of the diff there is lack of SHA-1 headers --- which is
unavoidable
> > > > until editor is revved --- but part of it is a missing
Text-copy-source-
> md5.
> > > > Why don't you output that information --- doesn't the editor give it
to
> you?
> > >
> > > Afaik, no. I don't see Text-copy-source-* anywhere in the RA
> > > layer. Maybe I'm not looking hard enough?
> > >
> >
> > Hmm.  It seems you're right.  So you might have to use two RA session in
> > parallel...
> >
> > (and then, you might have to have the user authenticate twice?)
> 
> Hm, I also have to find out if it's allowed. The commit_editor doesn't
> allow it for instance. Besides, it's a very inelegant solution- I'd
> rather fix the RA layer than do this.

@Daniel, what would adding these adders add?

The extra headers are for making it easier to detect corruptions by checking
them along the transfer. 

If we are just doing additional work to add headers via a different process
it slows the dumping down more than a bit and it doesn't make the dump file
any safer because it uses a different processes to obtain the header. 
I think you would have to obtain the source of the copyfrom and get some
checksum from that; maybe you can do that without transferring the file
again, but I'm not sure about that.

(And without the added headers the process is already as safe as svnsync.).

Yes, we can add more and more processing to also get those new Sha1 headers
by recalculating them while dumping, but the idea for svnrdump was to create
a fast and secure way to dump and load repositories... not an incredible
slow one that has to transfer files multiple times just to make all the
optional headers match the output of svnadmin.

Those headers were made optional for a reason: you don't always have them. 
And different conversion processes have different headers available.
Svnadmin looks at the FS layer for dumping, so it sees different things than
an RA layer api. E.g. the dump in svnadmin has to create diffs from
fulltexts itself, while svnrdump has diffs and must apply these itself to
get full texts. The checksums have a similar mangling. The FS has access to
some of the checksums and recalculates others for you. (See the performance
drop in 1.6 of svnadmin dump)

There is a similar case at the import side. Applying commits can't check all
the checksums, but the really important ones are already handled. Svnrdump
dump and svnrdump load are a nice match.

Bert

Re: svnrdump: The BIG update

[Ramkumar Ramachandra]

Hi Daniel,

Daniel Shahaf writes:
> Ramkumar Ramachandra wrote on Thu, Aug 12, 2010 at 12:17:34 +0530:
> > > > The dump functionality is also complete- thanks to Stefan's review and
> > > > MANY others for cleaning it up. It's however hit a brick wall now
> > > > because of missing headers in the RA layer. Until I (or someone else)
> > > > figures out how to fix the RA layer, we can't do better than the XFail
> > > > copy-and-modify test I've committed.
> > > 
> > > Part of the diff there is lack of SHA-1 headers --- which is unavoidable
> > > until editor is revved --- but part of it is a missing 
> > > Text-copy-source-md5.
> > > Why don't you output that information --- doesn't the editor give it to 
> > > you?
> > 
> > Afaik, no. I don't see Text-copy-source-* anywhere in the RA
> > layer. Maybe I'm not looking hard enough?
> > 
> 
> Hmm.  It seems you're right.  So you might have to use two RA session in
> parallel...
> 
> (and then, you might have to have the user authenticate twice?)

Hm, I also have to find out if it's allowed. The commit_editor doesn't
allow it for instance. Besides, it's a very inelegant solution- I'd
rather fix the RA layer than do this.

> > > > - Make dumpfile v3 the de-facto standard and improve it for optimized
> > > >   loading/ generation. The former part was suggested by Stefan.
> > > > - Integrate it into svnadmin etc as appropriate. I think there's
> > > >   enough work here for a mini-GSoC project?
> > > 
> > > How would it be integrated into svnadmin?  Do you want to push the logic
> > > into the standard 'svnadmin dump' command?
> > 
> > This is something I haven't given thought either. I brought it up
> > because of an earlier discussion in which everyone seemed to be in
> > favor of NOT having a new command. It feels like we're stuffing a lot
> > of functionality into one tool though.
> > 
> 
> Personally I also like having svnadmin operates only locally (so it doesn't
> even link against libsvn_ra), but that was hashed out already on that
> moderately-long thread a few weeks ago.

Yeah. It looks like I'll have to ressurect this thread soon and reach
a concrete conclusion.

-- Ram

Re: Obsolete URL on subversion.tigris.org

[Hyrum K. Wright]

Thanks.  Committed a fix to the tigris repo in r110.

For those wondering, the correct link is:
http://subversion.apache.org/docs/release-notes/release-history.html

-Hyrum

On Tue, Aug 17, 2010 at 10:40 AM, Vincent Lefevre
 wrote:
> Hi,
>
> When one searches for Subversion history on Google, one gets:
>
>  http://subversion.tigris.org/release-history.html
>
> This page says:
>
>  This information has been moved to
>  http://subversion.apache.org/release-history.html
>
> but here one gets a 404 Not Found error.
>
> The correct URL should be given, or there should be a redirection
> for "http://subversion.apache.org/release-history.html";.
>
> --
> Vincent Lefèvre  - Web: 
> 100% accessible validated (X)HTML - Blog: 
> Work: CR INRIA - computer arithmetic / Arénaire project (LIP, ENS-Lyon)
>

Obsolete URL on subversion.tigris.org

[Vincent Lefevre]

Hi,

When one searches for Subversion history on Google, one gets:

  http://subversion.tigris.org/release-history.html

This page says:

  This information has been moved to
  http://subversion.apache.org/release-history.html

but here one gets a 404 Not Found error.

The correct URL should be given, or there should be a redirection
for "http://subversion.apache.org/release-history.html";.

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / Arénaire project (LIP, ENS-Lyon)

Re: Looking to improve performance of svn annotate

[Greg Hudson]

On Tue, 2010-08-17 at 09:26 -0400, Johan Corveleyn wrote:
> Greg, could you explain a bit more what you mean with
> "edit-stream-style binary diffs", vs. the binary deltas we have now?
> Could you perhaps give an example similar to Julian's? Wouldn't you
> have the same problem with pieces of the source text being copied
> out-of-order (100 bytes from the end/middle of the source being copied
> to the beginning of the target, followed by the rest of the source)?

Let's take a look at the differences between a line-based edit stream
diff (such as what you'd see in the output of diff -e) and a binary
delta as we have in Subversion.

The most obvious difference is that the former traffics in lines, rather
than arbitrary byte ranges, but the actual differences are much deeper.
A line-based diff can be modeled with the following "instructions":

  * Copy the next N lines of source to target.
  * Skip the next N lines of source.
  * Copy the next N lines of new data to target.

After applying a diff like this, you can easily divide the target lines
into two categories: those which originated from the source, and those
which originated from the diff.  The division may not accurately
represent the intent of the change (there's the classic problem of the
mis-attributed close brace, for instance; see
http://bramcohen.livejournal.com/73318.html), but it's typically pretty
close.

Subversion binary deltas have a more flexible instruction set, more akin
to what you'd find in a compression algorithm.  The source and target
are chopped up into windows, and for each window you have:

  * Copy N bytes from offset O in the source window to target.
  * Copy N bytes from offset O in the target window to target.
  * Copy the next N bytes of new data to target.

There is no easy way to divide the target into source bytes and diff
bytes.  Certainly, you can tag which bytes were copied from the source
window, but that's meaningless.  Bytes which came from the source window
may have been rearranged by the diff; bytes which came from new data may
only have done so because of windowing.

The optimization idea is to create a new kind of diff (or more likely,
research an existing algorithm) which obeys the rules of the line-based
edit stream--no windowing, sequential access only into the source
stream--but traffics in bytes instead of lines.  With such a diff in
hand, we can divide the target bytes into source-origin and diff-origin,
and then, after splitting the target into lines, determine which lines
are "tainted" by diff-origin bytes and therefore should be viewed as
originating in the diff.

Re: svn commit: r106 - trunk/www: . cn-project-pages/snippets

[C. Michael Pilato]

I've handled this reversion and reworked the text there to make the point
that I believe Jack (in his role as a Tigris administrator) was mostly
trying to make:  "Don't look for new stuff on this, our old site, and then
complain to the Tigris admins when you can't find it."


On 08/17/2010 12:08 AM, Daniel Shahaf wrote:
> -1 on this change: it adds a link to openCollabNet to our tigris HOMEPAGE,
> without any prior discussion.
> 
> jrepenning_cn: please revert this asap, and avoid making changes to our
> Website in this manner.
> 
> jrepenning...@tigris.org wrote on Mon, Aug 16, 2010 at 17:24:52 -0700:
>> Author: jrepenning_cn
>> Date: 2010-08-16 17:24:51-0700
>> New Revision: 106
>>
>> Added:
>>trunk/www/cn-project-pages/snippets/
>>trunk/www/cn-project-pages/snippets/page.xml
>> Modified:
>>trunk/www/index.html
>>
>> Log:
>> Updated html component through the web interface.


-- 
C. Michael Pilato 
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand



signature.asc
Description: OpenPGP digital signature

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

[Bert Huijben]

> -Original Message-
> From: e...@apache.org [mailto:e...@apache.org]
> Sent: dinsdag 17 augustus 2010 7:43
> To: comm...@subversion.apache.org
> Subject: svn commit: r986332 - in /subversion/trunk/subversion/libsvn_wc:
> wc-queries.sql wc_db.c
> 
> Author: ehu
> Date: Tue Aug 17 14:42:42 2010
> New Revision: 986332
> 
> URL: http://svn.apache.org/viewvc?rev=986332&view=rev
> Log:
> Regarding NODE_DATA, split an INSERT query - for which the fields get
> spread
> over 2 tables - across 2 queries wrapped in a transaction. Mark another one
> which needs the same treatment.
> 
>  * subversion/libsvn_wc/wc-queries.sql
>   (STMT_INSERT_WORKING_NODE_DATA_FROM_BASE_NODE_1,
>STMT_INSERT_WORKING_NODE_DATA_FROM_BASE_NODE_2): Split from
>  STMT_INSERT_WORKING_NODE_FROM_BASE_NODE
>   (STMT_APPLY_CHANGES_TO_BASE): Mark for split up.
> 
> Modified:
> subversion/trunk/subversion/libsvn_wc/wc-queries.sql
> subversion/trunk/subversion/libsvn_wc/wc_db.c

Your log message doesn't describe any changes in wc_db.c

Bert

Re: Looking to improve performance of svn annotate

[Johan Corveleyn]

On Thu, Aug 12, 2010 at 5:30 PM, Greg Hudson  wrote:
> On Thu, 2010-08-12 at 10:57 -0400, Julian Foad wrote:
>> I'm wary of embedding any client functionality in the server, but I
>> guess it's worth considering if it would be that useful.  If so, let's
>> take great care to ensure it's only lightly coupled to the core server
>> logic.
>
> Again, it's possible that binary diffs between sequential revisions
> could be used for blame purposes (not the binary deltas we have now, but
> edit-stream-style binary diffs), which would decouple the
> line-processing logic from the server.
>
> (But again, I haven't thought through the problem in enough detail to be
> certain.)

If such edit-stream-style binary diffs could do the job, and they are
"fast enough" (I'm guessing that line based vs. binary wouldn't make
that much of a difference for the eventual blame processing), it seems
like a good compromise: we get the performance benefits of
blame-oriented delta's (supposedly fast and easy to calculate blame
info from), possibly cached on the server, while still not introducing
unnecessary coupling of the server to line-processing logic.

Greg, could you explain a bit more what you mean with
"edit-stream-style binary diffs", vs. the binary deltas we have now?
Could you perhaps give an example similar to Julian's? Wouldn't you
have the same problem with pieces of the source text being copied
out-of-order (100 bytes from the end/middle of the source being copied
to the beginning of the target, followed by the rest of the source)?
Wouldn't you also have to do the work of discovering the largest
contiguous block of source text as "the main stream", so determine
that those first 100 bytes are to be interpreted as new bytes, etc?

Caching this stuff on the server would of course be ideal. Whether it
be "post-commit" or on-demand (first guy requesting the blame takes
the hit), both approaches seem good to me. Working on that would be
severely out of my league though :-). At least for now.


Another thing that occurred to me: since most time of the current
blame implementation is spent on "diff" (svn_diff_file_diff_2), maybe
a quick win could be to simply (?) optimize the diff code? Or write a
specialized faster version for blame.

On my tests with a 1,5 Mb file (61000 lines), svn diffing it takes
about 500 ms on my machine. GNU diff is much faster (300 ms for the
first run, 72 ms on following runs). This seems to indicate that there
is much room for optimization of svn diff. Or is there something extra
that svn diff does, necessary in the svn context?

I have looked a little bit at the svn diff code, and saw that most of
the time is spent in the while loop inside svn_diff__get_tokens in
token.c, presumably extracting the tokens (lines) from the file(s).
Haven't looked any further/deeper. Anybody have any brilliant
ideas/suggestions? Or is this a bad idea, not worthy of further
exploration :-) ?

BTW, I also tested with Stefan Fuhrmann's performance bra...@r985697,
just for kicks (had some trouble building it on Windows, but
eventually managed to get an svn.exe out of it). The timing of svn
diff of such a large file was about the same, so that didn't help. But
maybe the branch isn't ready for prime time just yet ...

Cheers,
-- 
Johan

Re: svn commit: r106 - trunk/www: . cn-project-pages/snippets

[Daniel Shahaf]

-1 on this change: it adds a link to openCollabNet to our tigris HOMEPAGE,
without any prior discussion.

jrepenning_cn: please revert this asap, and avoid making changes to our
Website in this manner.

jrepenning...@tigris.org wrote on Mon, Aug 16, 2010 at 17:24:52 -0700:
> Author: jrepenning_cn
> Date: 2010-08-16 17:24:51-0700
> New Revision: 106
> 
> Added:
>trunk/www/cn-project-pages/snippets/
>trunk/www/cn-project-pages/snippets/page.xml
> Modified:
>trunk/www/index.html
> 
> Log:
> Updated html component through the web interface.
> 
> Added: trunk/www/cn-project-pages/snippets/page.xml
> Url: 
> http://subversion.tigris.org/source/browse/subversion/trunk/www/cn-project-pages/snippets/page.xml?view=markup&pathrev=106
> ==
> --- (empty file)
> +++ trunk/www/cn-project-pages/snippets/page.xml  2010-08-16 17:24:51-0700
> @@ -0,0 +1,16 @@
> +
> +
> +  
> +
> +  
> +
> +
> +  
> +  index.html
> +
> +
> +  Subprojects
> +
> +  
> +
> +
> 
> Modified: trunk/www/index.html
> Url: 
> http://subversion.tigris.org/source/browse/subversion/trunk/www/index.html?view=diff&pathrev=106&r1=105&r2=106
> ==
> --- trunk/www/index.html  (original)
> +++ trunk/www/index.html  2010-08-16 17:24:51-0700
> @@ -1,44 +1 @@
> - -"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd";>
> -http://www.w3.org/1999/xhtml";>
> -
> -subversion.tigris.org
> -
> -
> -#projecthome .axial { display: none; }
> -#apphead h1 { display: none; }
> -#longdescription { border: none; }
> -#longdescription h2 { display: none; }
> -#customcontent h2 { display: block; }
> -
> -
> -
> -
> -
> -
> -
> -  
> -
> -
> -
> -
> -This is the former website of the Subversion software project,
> -   which now calls http://subversion.apache.org/";
> -   style="font-weight: bold;" >subversion.apache.org home.
> -
> -Until the transition into Apache-hood is complete, this site will
> -   continue to serve some of the purposes of the Subversion project.
> -   For example, the project is still using
> -   the http://subversion.tigris.org/issue-tracker.html";
> -   >issue tracker hosted here.  But in time, it is expected that
> -   this site will be converted into mostly just a set of pointers to
> -   information that has moved over to the subversion.apache.org
> -   site.
> -
> -Thanks for your patience as we work through this transition!
> -
> -
> -
> - 
> -
> -
> +
>  
>
>   This is the former website of the Subversion software 
> project,which now calls  href="http://subversion.apache.org/";>subversion.apache.org home.  
> Until the transition into Apache-hood is complete, this site will
> continue to serve some of the purposes of the Subversion project.For 
> example, the project is still usingthe  href="http://subversion.tigris.org/issue-tracker.html";>issue tracker 
> hosted here.  But in time, it is expected thatthis site will be converted 
> into mostly just a set of pointers toinformation that has moved over to 
> the subversion.apache.orgsite.If you're looking for 
>  downloads of Subversion, this is the wrong place. There are some out-of-date 
> downloads here, for archival purposes, but for up-to-date installers, go to 
> http://www.open.collab.net/downloads/subversion/";>openCollabNet 
> , or one of the other sites listed at subversion.apache.org.  
>  Thanks for your patience as we work through this transition!
>
> \ No newline at end of file
> 
> --
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2647587