Re: Some tips on profiling
On 30.09.2010 04:28, Ramkumar Ramachandra wrote: Hi Stefan, Stefan Fuhrmann writes: My measurements seem to support what Philip wrote: The expensive part is run on the server. Even with my optimized server, the svnrdump CPU usage is less than the time taken by the server. Some numbers (hot file cache): svnadmin dump 1.7 trunk 70s real 66s user 4s system perf-branch 30s real 28s user 2s system 1.7 trunk svnrdump ra-local 88s real 81s user 7s system svn: (1.7 trunk) 99s real 6s user 4s system svn: (perf-branch, cold) 72s real 5s user 6s system svn: (perf-branch, hot) 17s real 5s user 5s system Thus, svnrdump is slower only for ra-local where it is of no particular use in the first place. To really speed things up, the caching infrastructure from the performance branch should be merged into /trunk. Wow, that looks awesome. Yeah, Daniel suggested that I run svnrdump against your perf branch yesterday, but I wasn't able to get your branch to build: subversion/libsvn_subr/svn_file_handle_cache.c:890: error: 'svn_file_handle_cache_t' has no member named 'mutex' You obviously don't have APR threads enabled. Thanks for finding this. Fixed in r1003430. What exactly are the changes that hot introduces- can you point me to the specific revision numbers that we intend to merge? The server caches almost everything. So, the first (cold) run demonstrates the worst-case, the second run (hot) the best case. Real world performance depends on server memory and load. For the merge part: please review the integrate-membuffer branch (only 3 patches). You may also review and merge any of the patches listed in /branches/performance/STATUS. @Stefan: Thoughts on hacking APR hashtables directly? Are you sure?! Which operation is the most expensive one and how often is it called? Who calls it and why? My bad. I got muddled up when using ra_local: when I saw lots of MD5 functions, I assumed it had to do something with the hashtable since the checksum was caculated by server-side. The profiler profiled both server-side and client-side. Over svn://, I got a much cleaner output. The new results indicate: 1. Server-client IO is the major bottleneck, as Philip and you pointed out. 2. On the client side, the major bottlneck is the IO during textdelta application. David (CC'ed now) and I are trying some experiments with a single temporary file. Thoughts? For the MD5 stuff, try r986459 (performance branch). It should eliminate 1 of the 3 MD5 calculations. As for the temp files, there are some improvements listed in /branches/performance/STATUS. These would also reduce your system CPU time. -- Stefan^2.
Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed
Daniel Trebbien wrote:
Following Daniel Shahaf's suggestion of splitting the allow svnsync
to translate non-UTF-8 log messages to UTF-8 work into two patches, I
have revised my changes, prepared one of the two patches, and have
attached the one patch and suggested log message to this e-mail.
[[[
Adds a public API function, svn_subst_translate_string2(), an extension of
svn_subst_translate_string(), that has two, additional output parameters for
determining whether re-encoding and/or line ending translation were performed.
If we're going to add this functionality to _translate_string(),
shouldn't we also add it to the other variants - _detranslate_string,
_translate_cstring2, _stream_translated, _copy_and_translate4?
I see you're adding the infrastructure into the (new) bodies of
_translate_cstring2 and _stream_translated, just not (yey) exposing it
in their APIs.
I'm not sure. The set of 'translation' functions is already messy and
it's not clear to me how useful this new functionality will be.
I wonder if any of the current users of these functions need to know
whether any change was made, and currently find out by executing a full
text comparison of the result. If so, then this enhancement should
allow us to simplify those callers.
As discussed at:
http://thread.gmane.org/gmane.comp.version-control.subversion.user/100020
http://thread.gmane.org/gmane.comp.version-control.subversion.devel/122518
http://thread.gmane.org/gmane.comp.version-control.subversion.devel/122550
* subversion/include/svn_subst.h
(svn_subst_translate_string2): New function.
(svn_subst_translate_string): Deprecated in favor of
svn_subst_translate_string2().
* subversion/libsvn_subr/subst.c
(translate_newline): Added the new parameter EOL_TRANSLATED, the value at
which is set to TRUE if translate_newline() wrote out the line ending in
NEWLINE_BUF with a different line ending in EOL_STR.
(translation_baton): Added the TRANSLATED_EOL field.
(create_translation_baton): Added a new parameter TRANSLATED_EOL that is
passed to the resulting translation_baton.
(translate_chunk): Modified the three calls to translate_newline() to pass
the TRANSLATED_EOL pointer from the translation baton.
(stream_translated): New static function. Its implementation is the old
implementation of svn_subst_stream_translated(), but accepting another
parameter, TRANSLATED_EOL, that is passed to the in/out translation batons
that it creates.
(svn_subst_stream_translated): Now a wrapper for stream_translated().
(translate_cstring2): New static function. Its implementation is the old
implementation of svn_subst_translate_cstring2(), but modified to accept
another parameter, TRANSLATED_EOL, that is passed to stream_translated().
(svn_subst_translate_cstring2): Now a wrapper for translate_cstring2().
(svn_subst_translate_string2): New function. The task of recording whether
it translated a line ending is delegated to translate_cstring2().
]]]
Index: subversion/libsvn_subr/subst.c
===
--- subversion/libsvn_subr/subst.c (revision 1003341)
+++ subversion/libsvn_subr/subst.c (working copy)
@@ -628,7 +628,8 @@ translate_newline(const char *eol_str,
char *newline_buf,
apr_size_t newline_len,
svn_stream_t *dst,
- svn_boolean_t repair)
+ svn_boolean_t repair,
+ svn_boolean_t *eol_translated)
{
Please update the doc string of this function to mention the new
parameter, or write a partial doc string if it doesn't have one. (Hmm,
I see it does have one but it's quite inaccurate. I will update the
current version now.)
@@ -650,7 +651,13 @@ translate_newline(const char *eol_str,
*src_format_len = newline_len;
}
/* Translate the newline */
- return translate_write(dst, eol_str, eol_str_len);
+ svn_error_t *err = translate_write(dst, eol_str, eol_str_len);
No declarations mixed in with statements - we stick to C'89 rules. But
I don't think there is any need to insert this new code *after* the
write - it can just as well go before the write, leaving the 'return'
how it was.
+ if (eol_translated) {
+if (newline_len != eol_str_len ||
+strncmp(newline_buf, eol_str, newline_len))
+ *eol_translated = TRUE;
+ }
+ return err;
}
At first I was concerned that this is a lot of processing overhead, but
then I realised that the only real-world case that would execute the
'strncmp' every time would be translating files from the old Mac format
'\r' to '\n', which I imagine is extremely uncommon these days.
@@ -765,6 +772,7 @@ svn_subst_keywords_differ2(apr_hash_t *a,
struct translation_baton
{
const char *eol_str;
+ svn_boolean_t *translated_eol;
svn_boolean_t repair;
Let's use the same name in each place:
RE: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed
-Original Message-
From: Julian Foad [mailto:julian.f...@wandisco.com]
Sent: vrijdag 1 oktober 2010 12:58
To: Daniel Trebbien
Cc: Subversion Development; Daniel Shahaf; Gavin Beau Baumanis
Subject: Re: [PATCH] extend svn_subst_translate_string() to record
whether re-encoding and/or line ending translation were performed
[Copying from this mail, as it already quotes the sources that are separate
attachments for me]
Please update the doc string of this function to mention the new
parameter, or write a partial doc string if it doesn't have one. (Hmm,
I see it does have one but it's quite inaccurate. I will update the
current version now.)
@@ -650,7 +651,13 @@ translate_newline(const char *eol_str,
*src_format_len = newline_len;
}
/* Translate the newline */
- return translate_write(dst, eol_str, eol_str_len);
+ svn_error_t *err = translate_write(dst, eol_str, eol_str_len);
No declarations mixed in with statements - we stick to C'89 rules. But
I don't think there is any need to insert this new code *after* the
write - it can just as well go before the write, leaving the 'return'
how it was.
The code can just use SVN_ERR() here, as you can't be sure the output is
available in error conditions anyway, so the extra check can be avoided on
errors.
+ if (eol_translated) {
+if (newline_len != eol_str_len ||
+strncmp(newline_buf, eol_str, newline_len))
+ *eol_translated = TRUE;
+ }
+ return err;
And this can be a return SVN_NO_ERROR;
}
Bert
Re: disallow merges into mixed-rev WCs by default?
Stefan Sperling wrote: I'd like to propose that we disallow merges into mixed-revision working copies by default for 1.7, and only allow such merges if a special command line option has been passed. The reason for this is that merges into mixed-rev WCs can easily create conflicts which are tedious to resolve. Even if you know about the problem and try to run svn update before every merge, it's quite easy to forget about running svn update. Also, we've been advertising this as best practice for some time. Quoting the book on the matter: Never merge into working copies with a mixture of working revision numbers, or with “switched” subdirectories (as described next in the section called “Traversing Branches”). A merge target should be a working copy which represents a single location in the repository at a single point in time. Note that while the book recommends not to merge into working copies with switched subtrees, I don't intend to enforce this by default. I've talked about this with Paul on IRC and he had no objections. See http://colabti.org/irclogger/irclogger_log/svn-dev?date=2010-09-30#l468 for the full discussion. Below is a work-in-progress diff that starts making this change. Tests fail with it. I've only checked the merge tests for now. Before continuing with this, fixing up tests, bumping APIs, and eventually adding the command line option that allows merges into mixed-rev WCs, I'd like to ask whether anyone has concerns about this idea. The proposal sounds good to me. To be more specific about what the proposal is: * It does not apply to re-integrate merges. (They always enforce this anyway?) * It applies to merge-tracking merges. * It [applies/does not appy] to non-merge-tracking merges? As for the tests: I think it would be best to keep some or all of the tests doing merges into a mixed-rev WC as they do now, partly so as not to lose test coverage of this condition, but more importantly to preserve the purpose of any regression tests that happen to be testing for bugs which only showed up in mixed-rev mode. In other words, fix most of them by adding the bypass flag rather than by adding an svn update command. - Julian Below is the list of currently failing merge tests, and the work-in-progress diff. Thanks, Stefan FAIL: merge_tests.py 1: performing a merge, with mixed results FAIL: merge_tests.py 9: merge change into unchanged binary file FAIL: merge_tests.py 15: merge should skip over unversioned obstructions FAIL: merge_tests.py 16: merge into missing must not break working copy FAIL: merge_tests.py 23: merge between branches (Issue #) FAIL: merge_tests.py 31: merge a replacement of a file to mixed rev wc FAIL: merge_tests.py 32: ignore whitespace when merging FAIL: merge_tests.py 33: ignore eolstyle when merging FAIL: merge_tests.py 44: merge to path with switched children FAIL: merge_tests.py 69: subtrees added after start of merge range are ok FAIL: merge_tests.py 70: cyclic merges don't add mergeinfo from own history FAIL: merge_tests.py 77: merges that remove all mergeinfo work FAIL: merge_tests.py 89: target and subtrees need nonintersecting revs FAIL: merge_tests.py 90: merge two successive edits to the same property FAIL: merge_tests.py 91: merge an EOL unification and set svn:eol-style FAIL: merge_tests.py 103: automatic conflict resolutions work with merge FAIL: merge_tests.py 105: committed case only move causes revert to fail FAIL: merge_tests.py 107: merge del and ps variants from a foreign repos FAIL: merge_tests.py 111: merge into locally added file FAIL: merge_tests.py 112: merge into locally added directory Index: subversion/libsvn_client/merge.c === --- subversion/libsvn_client/merge.c (revision 1003282) +++ subversion/libsvn_client/merge.c (working copy) @@ -8815,6 +8815,54 @@ merge_cousins_and_supplement_mergeinfo(const char return SVN_NO_ERROR; } +/* Perform checks to determine whether of the working copy at TARGET_ABSPATH + * can safely be used as a merge target. Checks are performed according to + * the FORBID_MIXED_REV, FORBID_LOCAL_MODS, and FORBID_SWITCHED_SUBTREES + * parameters. If any checks fail, raise SVN_ERR_CLIENT_NOT_READY_TO_MERGE. + * + * E.g. if all the FORBID_* parameters are TRUE, TARGET_ABSPATH must + * be a single-revision, pristine, unswitched working copy. + * In other words, it must reflect a subtree of the repostiory as found + * at single revision -- although sparse checkouts are permitted. */ +static svn_error_t * +ensure_wc_is_suitable_merge_target(const char *target_abspath, + svn_client_ctx_t *ctx, + svn_boolean_t forbid_mixed_rev, + svn_boolean_t forbid_local_mods, + svn_boolean_t
Re: disallow merges into mixed-rev WCs by default?
On Fri, Oct 1, 2010 at 7:25 AM, Julian Foad julian.f...@wandisco.com wrote: Stefan Sperling wrote: I'd like to propose that we disallow merges into mixed-revision working copies by default for 1.7, and only allow such merges if a special command line option has been passed. What will the behavior be during --dry-run merges? A) No change B) Warn that your target is mixed-rev, but proceed as we always did C) Fail unless the new option is used. What about --record-only merges? I think here it makes sense to mirror whatever behavior we decide upon for a real merge-tracking aware merge. The reason for this is that merges into mixed-rev WCs can easily create conflicts which are tedious to resolve. Even if you know about the problem and try to run svn update before every merge, it's quite easy to forget about running svn update. Also, we've been advertising this as best practice for some time. Quoting the book on the matter: Never merge into working copies with a mixture of working revision numbers, or with “switched” subdirectories (as described next in the section called “Traversing Branches”). A merge target should be a working copy which represents a single location in the repository at a single point in time. Note that while the book recommends not to merge into working copies with switched subtrees, I don't intend to enforce this by default. I've talked about this with Paul on IRC and he had no objections. See http://colabti.org/irclogger/irclogger_log/svn-dev?date=2010-09-30#l468 for the full discussion. Below is a work-in-progress diff that starts making this change. Tests fail with it. I've only checked the merge tests for now. Before continuing with this, fixing up tests, bumping APIs, and eventually adding the command line option that allows merges into mixed-rev WCs, I'd like to ask whether anyone has concerns about this idea. The proposal sounds good to me. To be more specific about what the proposal is: * It does not apply to re-integrate merges. (They always enforce this anyway?) Right, there should be *no* override on reintegrate's requirement for a uniform rev target. * It applies to merge-tracking merges. * It [applies/does not appy] to non-merge-tracking merges? Stefan and I didn't discuss these last two. I'm inclined to think that this warning should apply only to merge-tracking aware merges. As for the tests: I think it would be best to keep some or all of the tests doing merges into a mixed-rev WC as they do now, partly so as not to lose test coverage of this condition, but more importantly to preserve the purpose of any regression tests that happen to be testing for bugs which only showed up in mixed-rev mode. In other words, fix most of them by adding the bypass flag rather than by adding an svn update command. Good point, that makes more sense. Paul
Re: 1.6.13 up for signing/testing
Hyrum K. Wright wrote: 1.6.13 tarballs are up for testing and signing. The magic revision is r1002816: http://people.apache.org/~hwright/svn/1.6.13/ Summary: +1 to release (Unix) Tested: 1.6.13 tarball with local dependencies (nothing from deps tarball) [ra_local, ra_svn, ra_serf, ra_neon] x [fsfs, bdb] Dependencies: APR 1.3.9 APR-UTIL 1.3.9 Apache httpd 2.2.14 Neon 0.29.3 Serf 0.3.0 BDB 4.7.25 OpenSSL 0.9.8g SunJDK 1.6.0_12 SQLite 3.6.19 JUnit 3.8.2 Perl 5.10.0 Python 2.5.2 Ruby 1.8.7 Swig 1.3.36 ctypesgen trunk Environment: Debian 5.0.6 (Lenny) Linux 2.6.26-2-686 #1 SMP i686 GNU/Linux Results: All tests PASSED! Bindings results: javahl Time: 259.061 OK (50 tests) perl All tests successful, 7 subtests skipped. python Ran 106 tests in 62.608s ruby 219 tests, 1494 assertions, 0 failures, 0 errors ctypes-python ...Ran 26 tests in 30.600s (OK) md5sums (tested): 7ae1c827689f21cf975804005be30aeb subversion-1.6.13.tar.bz2 8451f5d771edc0a0302bd9a52d54e150 subversion-1.6.13.tar.gz Verfied md5sums (nothing more): 2a7d662bac872c61a5e11c89263d7f07 subversion-deps-1.6.13.tar.bz2 688bdb107731f9db2f3b6297b663a68d subversion-deps-1.6.13.tar.gz 88dc208617bf8ed4abbe46953cd0d387 svn_version.h.dist GPG Signatures: subversion-1.6.13.tar.bz2.asc -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) iEYEABECAAYFAkylqIkACgkQ9o1G+2zNQDh/oQCcCK7wSFSGZcjANEGg2tmThFom b0sAn1bZIFjRY/+JUmJ5LFuCZwL/rtdd =rJdy -END PGP SIGNATURE- subversion-1.6.13.tar.gz.asc -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) iEYEABECAAYFAkylqJAACgkQ9o1G+2zNQDgF8ACgiWCHM7Hyc5xhguaknPAsNerQ n/4An2E9ASkvd8nHWjgyj1fnJ0P7FfGt =8Qpx -END PGP SIGNATURE- subversion-deps-1.6.13.tar.bz2.asc -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) iEYEABECAAYFAkylqJkACgkQ9o1G+2zNQDidwACgh0y6L4UYa6/I8ZpcdmpetIBp Y7QAnA5DL6mO0LrYCNEFqUu+Ft5YDo9F =U9h9 -END PGP SIGNATURE- subversion-deps-1.6.13.tar.gz.asc -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) iEYEABECAAYFAkylqJ0ACgkQ9o1G+2zNQDh/dwCgmtWchpQu7ywVffbGtR3H041R eSsAn2T0NLqdf2xPmqLQxaXLrnZIsu7F =Mo7K -END PGP SIGNATURE- Thank You. -- Senthil Kumaran S http://www.stylesen.org/
Re: trunk failing tests on Windows XP (32 bit): prop-tests.py 33, stat-tests.py 5, upgrade-tests.py 11
On Thu, Sep 30, 2010 at 8:10 PM, Bert Huijben b...@qqmail.nl wrote: -Original Message- From: Johan Corveleyn [mailto:jcor...@gmail.com] Sent: vrijdag 1 oktober 2010 1:51 To: Subversion Development Subject: trunk failing tests on Windows XP (32 bit): prop-tests.py 33, stat-tests.py 5, upgrade-tests.py 11 Hi devs, The following tests fail on my machine (Windows XP 32-bit, built with VCE 2008, ra_local): - prop-tests.py 33: test properties of obstructed subdirectories svn: Can't open directory 'C:\research\svn\client_build\svn_trunk\Release\subversion\tests\cmdlin e\svn-test-work\working_copies\prop_tests-33\A\C': The directory name is invalid. - stat-tests.py 5: status on versioned items whose type has changed svn: Can't open directory 'C:\research\svn\client_build\svn_trunk\Release\subversion\tests\cmdlin e\svn-test-work\working_copies\stat_tests-5\A': The directory name is invalid. - upgrade_tests.py 11: missing directories and obstructing files svn: Can't open directory 'C:\research\svn\client_build\svn_trunk\Release\subversion\tests\cmdlin e\svn-test-work\working_copies\upgrade_tests-11\A\D': The directory name is invalid. They all seem to try to open some path as a directory, but it isn't a directory (but an empty file or something). Am I the only one seeing this? Is this known/normal? This seems to be related to the APR version you use. (Paul Burba reported this same problem earlier this week). Hi Johan, Bert is correct, I saw these failure earlier this week while using APR 1.3.12. All the failures occur when a file unexpectedly obstructs a directory. The APR macro APR_STATUS_IS_ENOTDIR in 1.3.x does not handle the ERROR_DIRECTORY 267 The directory name is invalid that Windows raises in this case. This was fixed in APR in http://svn.apache.org/viewvc?view=revisionrevision=821306 and is in APR 1.4.2. Paul
Re: Some tips on profiling
Hi Stefan, Stefan Fuhrmann writes: You obviously don't have APR threads enabled. Thanks for finding this. Fixed in r1003430. I enabled it, but there's still some issue: subversion/svnadmin/main.c:1892: undefined reference to `svn_fs_get_cache_config' The server caches almost everything. So, the first (cold) run demonstrates the worst-case, the second run (hot) the best case. Real world performance depends on server memory and load. Ah. Thanks for the clarification. For the merge part: please review the integrate-membuffer branch (only 3 patches). You may also review and merge any of the patches listed in /branches/performance/STATUS. integrate-cache-membuffer, you mean? Thanks! Your emails contains references exactly to the patches I'm looking for :) For the MD5 stuff, try r986459 (performance branch). It should eliminate 1 of the 3 MD5 calculations. Why doesn't STATUS document this and everything else consistently? As for the temp files, there are some improvements listed in /branches/performance/STATUS. These would also reduce your system CPU time. I had the chance to check them out and apply them just now. They work as expected. I'll submit some (uneducated) patch reviews to the list and request a merge. I don't have access to the areas your patches touch. -- Ram
[Merge request] Merge r985477 from performance branch
Hi,
I would like to get r985477 merged into trunk. I've applied and used
it successfully and checked that all tests pass.
Warning: I have no background knowledge. I'm just reviewing the patch
as-it-is, because Stefan asked me to.
[[[
r985477 | stefan2 | 2010-08-14 18:02:04 +0530 (Sat, 14 Aug 2010) | 9 lines
Eliminate all file system access in get_default_file_perms() for all but the
first execution.
The only downside is that we don't detect FS permission changes made while
the
(client) process runs. Because such changes would cause race conditions and
file I/O
errors anyway, we are not make things worse by omitting those tests.
* subversion/libsvn_subr/io.c
(get_default_file_perms): store result in a singleton in the first run and
bypass
the FS access in all later runs
I looked at this after reading the patch, and I must admit that I
didn't get the idea from the log message: how about using static
variable instead of singleton.
]]]
Index: subversion/libsvn_subr/io.c
===
--- subversion/libsvn_subr/io.c (revision 985476)
+++ subversion/libsvn_subr/io.c (revision 985477)
@@ -1297,30 +1297,47 @@ reown_file(const char *path,
static svn_error_t *
get_default_file_perms(apr_fileperms_t *perms, apr_pool_t *scratch_pool)
{
- apr_finfo_t finfo;
- apr_file_t *fd;
+ /* the default permissions as read from the temp folder */
+ static apr_fileperms_t default_perms = 0;
From the APR documentation, this is just an int32. Hm, you've got rid
of the file description and finfo -- let's see how this works out.
- /* Get the perms for a newly created file to find out what bits
- should be set.
+ /* Technically, this racy: Multiple threads may use enter here and
+ try to figure out the default permisission concurrently. That's fine
+ since they will end up with the same results. Even more technical,
+ apr_fileperms_t is an atomic type on 32+ bit machines.
+ */
+ if (default_perms == 0)
+{
+ apr_finfo_t finfo;
+ apr_file_t *fd;
Ah, so you didn't remove them.
- NOTE: normally del_on_close can be problematic because APR might
- delete the file if we spawned any child processes. In this case,
- the lifetime of this file handle is about 3 lines of code, so
- we can safely use del_on_close here.
+ /* Get the perms for a newly created file to find out what bits
+should be set.
- NOTE: not so fast, shorty. if some other thread forks off a child,
- then the APR cleanups run, and the file will disappear. sigh.
+NOTE: normally del_on_close can be problematic because APR might
+ delete the file if we spawned any child processes. In this case,
+ the lifetime of this file handle is about 3 lines of code, so
+ we can safely use del_on_close here.
Bogus diff due to re-indent. The only real addition is Get the perms
for a newly created file to find out what bits should be set.
- Using svn_io_open_uniquely_named() here because other tempfile
- creation functions tweak the permission bits of files they create.
- */
- SVN_ERR(svn_io_open_uniquely_named(fd, NULL, NULL, svn-tempfile, .tmp,
- svn_io_file_del_on_pool_cleanup,
- scratch_pool, scratch_pool));
- SVN_ERR(svn_io_file_info_get(finfo, APR_FINFO_PROT, fd, scratch_pool));
- SVN_ERR(svn_io_file_close(fd, scratch_pool));
+NOTE: not so fast, shorty. if some other thread forks off a child,
+ then the APR cleanups run, and the file will disappear. sigh.
Ok. You've moved this comment down to say why the del_on_close idea in
the previous comment is a bad idea.
+ *perms = finfo.protection;
+ default_perms = finfo.protection;
Very simple patch. Basically, if the permissions were found in the
last function call, simply return the found value, using a static
variable for remembering it. Otherwise, re-calculate permissions.
-- Ram
Re: 1.6.13 up for signing/testing
On Wed, 2010-09-29, Hyrum K. Wright wrote: 1.6.13 tarballs are up for testing and signing. [...] Summary: +1 to release (Unix). Tested: [ bdb | fsfs ] x [ ra_local | ra_svn | ra_neon | ra_serf ] swig-pl swig-py swig-rb ctypes-python Environment: OS/Platform: Ubuntu 9.10, 2.6.31-22-generic i686 GNU/Linux Using in-tree build of subversion-deps-1.6.13 packages: (none) Using Ubuntu distribution-supplied packages: libapr1 1.3.8-1 libaprutil1 1.3.9+dfsg-1ubuntu1 libdb-dev 4.7.25.3 openssl 0.9.8g-16ubuntu3.2 perl 5.10.0-24ubuntu4 python 2.6.4 ruby 1.8.7 python-ctypes 1.0.2-6ubuntu1 zlib1g 1:1.2.3.3.dfsg-13ubuntu3 neon 0.28.6 Results: make check ... (8 ways): No failures. make check-swig-pl: All tests successful, 7 subtests skipped. make check-swig-py: Ran 106 tests [...] OK. make check-swig-rb: 219 tests, 1494 assertions, 0 failures, 0 errors make check-ctypes-python: Ran 26 tests [...] OK. Paranoia: Verified that all 6 distribution packages match their md5sums, sha1sums and to-tigris copies. Verified that *.bz2 vs *.gz uncompress to exactly the same tarball, for both deps and non-deps. Signatures: ::: subversion-1.6.13.tar.bz2 ::: -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) iEYEABECAAYFAkylnb8ACgkQNR8z5DU+Jbzm3gCfUGZ35xVQVnxayCa6meyhJkea 1dgAn0vYuIXMKN5IGAAx6jAoVBbKrt6z =LPkD -END PGP SIGNATURE- ::: subversion-1.6.13.tar.gz ::: -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) iEYEABECAAYFAkylnb8ACgkQNR8z5DU+JbwDWACfXPaYXKdzH1j67G3SaWwrqFIl IbAAn3k2TVGzIpElHJiv7z/uI6f8TmaL =i5fp -END PGP SIGNATURE- - Julian
Re: disallow merges into mixed-rev WCs by default?
On Fri, 2010-10-01 at 09:16 -0400, Paul Burba wrote: On Fri, Oct 1, 2010 at 7:25 AM, Julian Foad julian.f...@wandisco.com wrote: Stefan Sperling wrote: I'd like to propose that we disallow merges into mixed-revision working copies by default for 1.7, and only allow such merges if a special command line option has been passed. What will the behavior be during --dry-run merges? A) No change B) Warn that your target is mixed-rev, but proceed as we always did C) Fail unless the new option is used. It should be (C): a dry-run should be as close as possible to the real behaviour. I can't see any benefit in making a dry-run merge behave differently from a non-dry-run merge in this respect. What about --record-only merges? I think here it makes sense to mirror whatever behavior we decide upon for a real merge-tracking aware merge. Agreed. The reason for this is that merges into mixed-rev WCs can easily create conflicts which are tedious to resolve. Even if you know about the problem and try to run svn update before every merge, it's quite easy to forget about running svn update. Also, we've been advertising this as best practice for some time. Quoting the book on the matter: Never merge into working copies with a mixture of working revision numbers, or with “switched” subdirectories (as described next in the section called “Traversing Branches”). A merge target should be a working copy which represents a single location in the repository at a single point in time. Note that while the book recommends not to merge into working copies with switched subtrees, I don't intend to enforce this by default. I've talked about this with Paul on IRC and he had no objections. See http://colabti.org/irclogger/irclogger_log/svn-dev?date=2010-09-30#l468 for the full discussion. Below is a work-in-progress diff that starts making this change. Tests fail with it. I've only checked the merge tests for now. Before continuing with this, fixing up tests, bumping APIs, and eventually adding the command line option that allows merges into mixed-rev WCs, I'd like to ask whether anyone has concerns about this idea. The proposal sounds good to me. To be more specific about what the proposal is: * It does not apply to re-integrate merges. (They always enforce this anyway?) Right, there should be *no* override on reintegrate's requirement for a uniform rev target. * It applies to merge-tracking merges. * It [applies/does not appy] to non-merge-tracking merges? Stefan and I didn't discuss these last two. I'm inclined to think that this warning should apply only to merge-tracking aware merges. I have no opinion on that. As for the tests: I think it would be best to keep some or all of the tests doing merges into a mixed-rev WC as they do now, partly so as not to lose test coverage of this condition, but more importantly to preserve the purpose of any regression tests that happen to be testing for bugs which only showed up in mixed-rev mode. In other words, fix most of them by adding the bypass flag rather than by adding an svn update command. Good point, that makes more sense. Paul - Julian
Re: [Merge request] Merge r985477 from performance branch
Ramkumar Ramachandra wrote:
I would like to get r985477 merged into trunk. I've applied and used
it successfully and checked that all tests pass.
Warning: I have no background knowledge. I'm just reviewing the patch
as-it-is, because Stefan asked me to.
[[[
r985477 | stefan2 | 2010-08-14 18:02:04 +0530 (Sat, 14 Aug 2010) | 9 lines
Eliminate all file system access in get_default_file_perms() for all but
the first execution.
The only downside is that we don't detect FS permission changes made while
the
(client) process runs. Because such changes would cause race conditions and
file I/O
errors anyway, we are not make things worse by omitting those tests.
Looks good to me.
I wondered if it is safe in a long-running Subversion process, like
TortoiseSvn or a Linux equivalent.
It seems to me that it won't really matter much in practice. If someone
changes their umask and finds that Subversion carries on creating files
with the 'old' permissions that were in effect when Subversion was
started... that's fine, as far as I'm concerned.
To make it easier for others to see, here's the patch, ignoring
indentation changes:
[[[
$ svn diff -x -upw -c985477 http://svn.apache.org/repos/asf/subversion
Index: branches/performance/subversion/libsvn_subr/io.c
===
--- branches/performance/subversion/libsvn_subr/io.c(revision 985476)
+++ branches/performance/subversion/libsvn_subr/io.c(revision 985477)
@@ -1297,6 +1297,16 @@
static svn_error_t *
get_default_file_perms(apr_fileperms_t *perms, apr_pool_t *scratch_pool)
{
+ /* the default permissions as read from the temp folder */
+ static apr_fileperms_t default_perms = 0;
+
+ /* Technically, this racy: Multiple threads may use enter here and
+ try to figure out the default permisission concurrently. That's fine
+ since they will end up with the same results. Even more technical,
+ apr_fileperms_t is an atomic type on 32+ bit machines.
+ */
+ if (default_perms == 0)
+{
apr_finfo_t finfo;
apr_file_t *fd;
@@ -1321,6 +1331,13 @@ get_default_file_perms(apr_fileperms_t *
SVN_ERR(svn_io_file_close(fd, scratch_pool));
*perms = finfo.protection;
+ default_perms = finfo.protection;
+}
+ else
+{
+ *perms = default_perms;
+}
+
return SVN_NO_ERROR;
}
]]]
- Julian
RE: 1.6.13 up for signing/testing
-Original Message- From: Julian Foad [mailto:julian.f...@wandisco.com] Sent: vrijdag 1 oktober 2010 16:51 To: Hyrum K. Wright Cc: Subversion Development Subject: Re: 1.6.13 up for signing/testing On Wed, 2010-09-29, Hyrum K. Wright wrote: 1.6.13 tarballs are up for testing and signing. [...] Summary: +1 to release (Windows) Tested: [ bdb | fsfs ] x [ ra_local | ra_svn | ra_neon | ra_serf ] Environment: Windows 7 X64, 32 bit tests. VS 2008 SP1 (Same dependencies as last time, except for serf 0.7.0) Bert == subversion-1.6.13.tar.bz2.asc == -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.16 (FreeBSD) iEYEABECAAYFAkymCjwACgkQ/P1jBZgh97IWVQCfUImkKsGhhtn9tt974/Q18vuR tuEAmwQ8LgFDvgzP1/hJinqXTCKkoQ3v =fxOQ -END PGP SIGNATURE- == subversion-1.6.13.tar.gz.asc == -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.16 (FreeBSD) iEYEABECAAYFAkymCkEACgkQ/P1jBZgh97JltQCffZ+C77+GGpeie4mZmd1PkLaZ WI8An2MbKty0PgpOkbkQkFeM7PZa/P/f =Qf5L -END PGP SIGNATURE- == subversion-1.6.13.zip.asc == -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.16 (FreeBSD) iEYEABECAAYFAkymCkUACgkQ/P1jBZgh97KknQCgqP/jucRamuT8cjDKbpqdGeyI EpsAn3Y74JUCqWBUDN7d0yvzQqqHaoyw =TO4A -END PGP SIGNATURE- == svn_version.h.dist.asc == -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.16 (FreeBSD) iEYEABECAAYFAkymCkoACgkQ/P1jBZgh97JMFgCfYHP+OoUQllvEuvGaecOvsr4b IPAAoMnPEoX6aQWgDPsocsudZcUsOz3H =0GM+ -END PGP SIGNATURE-
Subversion 1.6.13 Released
I'm happy to announce Subversion 1.6.13, available from: http://subversion.tigris.org/downloads/subversion-1.6.13.tar.bz2 http://subversion.tigris.org/downloads/subversion-1.6.13.tar.gz http://subversion.tigris.org/downloads/subversion-1.6.13.zip http://subversion.tigris.org/downloads/subversion-deps-1.6.13.tar.bz2 http://subversion.tigris.org/downloads/subversion-deps-1.6.13.tar.gz http://subversion.tigris.org/downloads/subversion-deps-1.6.13.zip This is a bugfix release, part of the 1.6.x release series. Of note, this release includes a fix which addresses CVE-2010-3313, a security issue when using 'SVNPathAuthz short_circuit'. More information can be found here: http://subversion.apache.org/security/CVE-2010-3313-advisory.txt The MD5 checksums are: 7ae1c827689f21cf975804005be30aeb subversion-1.6.13.tar.bz2 8451f5d771edc0a0302bd9a52d54e150 subversion-1.6.13.tar.gz dd4009b239a5354b434e5f66cddde145 subversion-1.6.13.zip 2a7d662bac872c61a5e11c89263d7f07 subversion-deps-1.6.13.tar.bz2 688bdb107731f9db2f3b6297b663a68d subversion-deps-1.6.13.tar.gz bb960d37f835e5e556e23e4eb85a9b08 subversion-deps-1.6.13.zip The SHA1 checksums are: 185efd129c3c4b04f1544d62bb9a3fcd0f58ba29 subversion-1.6.13.tar.bz2 06d3afc49182c80ea712c13409c008d27a4e889b subversion-1.6.13.tar.gz 6530528fae0335cd8495ebf1f2072e2dd9df2e31 subversion-1.6.13.zip e51bffda416a3a9abe068aab6f90d174dedef352 subversion-deps-1.6.13.tar.bz2 1faa5ba0c87210f534ed445d061e4955396524d4 subversion-deps-1.6.13.tar.gz 945ef9f68998aedf320af50d220e47a470684a06 subversion-deps-1.6.13.zip PGP Signatures are available at: http://subversion.tigris.org/downloads/subversion-1.6.13.tar.bz2.asc http://subversion.tigris.org/downloads/subversion-1.6.13.tar.gz.asc http://subversion.tigris.org/downloads/subversion-1.6.13.zip.asc http://subversion.tigris.org/downloads/subversion-deps-1.6.13.tar.bz2.asc http://subversion.tigris.org/downloads/subversion-deps-1.6.13.tar.gz.asc http://subversion.tigris.org/downloads/subversion-deps-1.6.13.zip.asc For this release, the following people have provided PGP signatures: Senthil Kumaran S [1024D/6CCD4038] with fingerprint: 8035 16A5 1D6E 50E2 1ECD DE56 F68D 46FB 6CCD 4038 Philip Martin [2048R/ED1A599C] with fingerprint: A844 790F B574 3606 EE95 9207 76D7 88E1 ED1A 599C Paul T. Burba [1024D/53FCDC55] with fingerprint: E630 CF54 792C F913 B13C 32C5 D916 8930 53FC DC55 Julian Foad [1024D/353E25BC] with fingerprint: 6604 5A4B 43BC F994 5728 351F 33E4 353E 25BC Bert Huijben [1024D/9821F7B2] with fingerprint: 2017 F51A 2572 0E78 8827 5329 FCFD 6305 9821 F7B2 Hyrum K. Wright [1024D/4E24517C] with fingerprint: 3324 80DA 0F8C A37D AEE6 D084 0B03 AE6E 4E24 517C Stefan Sperling [1024D/F59D25F0] with fingerprint: B1CF 1060 A1E9 34D1 9E86 D6D6 E5D3 0273 F59D 25F0 Mark Phippard [1024D/035A96A9] with fingerprint: D315 89DB E1C1 E9BA D218 39FD 265D F8A0 035A 96A9 Release notes for the 1.6.x release series may be found at: http://subversion.apache.org/docs/release-notes/1.6.html You can find the list of changes between 1.6.13 and earlier versions at: http://svn.apache.org/repos/asf/subversion/tags/1.6.13/CHANGES Questions, comments, and bug reports to us...@subversion.apache.org. Thanks, - The Subversion Team
Re: svn commit: r1002898 - in /subversion/trunk: ./ subversion/include/svn_string.h subversion/libsvn_subr/svn_string.c
On 10/01/2010 01:36 AM, Stefan Fuhrmann wrote: On 30.09.2010 02:12, Blair Zajac wrote: On 09/29/2010 05:01 PM, stef...@apache.org wrote: Author: stefan2 Date: Thu Sep 30 00:01:45 2010 New Revision: 1002898 URL: http://svn.apache.org/viewvc?rev=1002898view=rev Log: Merge r1001413 from the perform struct -- Stefan^2. ance branch. Improve documentation on svn_stringbuf_appendbyte. Approved by: Hyrum K. Wrighthyrum_wright_at_mail.utexas.edu /** Append a single character @a byte onto @a targetstr. + * This is an optimized version of @ref svn_stringbuf_appendbytes + * that is much faster to call and execute. Gains vary with the ABI. When you say ABI, you actually mean the C calling convention ABI, not the Subversion ABI? I think this would be a good distinction to document here, since Subversion has its own ABI that we strictly maintain. I mean calling convention. Where is the Subversion ABI defined? By ABI, I mean everything in subversion/include. We can't change function definitions or modify structures in such a way that breaks the ABI. Blair
Subversion 1.6.13 Released
[ Please note: I prior version of this mail referred to the incorrect CVE number. ] I'm happy to announce Subversion 1.6.13, available from: http://subversion.tigris.org/downloads/subversion-1.6.13.tar.bz2 http://subversion.tigris.org/downloads/subversion-1.6.13.tar.gz http://subversion.tigris.org/downloads/subversion-1.6.13.zip http://subversion.tigris.org/downloads/subversion-deps-1.6.13.tar.bz2 http://subversion.tigris.org/downloads/subversion-deps-1.6.13.tar.gz http://subversion.tigris.org/downloads/subversion-deps-1.6.13.zip This is a bugfix release, part of the 1.6.x release series. Of note, this release includes a fix which addresses CVE-2010-3315, a security issue when using 'SVNPathAuthz short_circuit'. More information can be found here: http://subversion.apache.org/security/CVE-2010-3315-advisory.txt The MD5 checksums are: 7ae1c827689f21cf975804005be30aeb subversion-1.6.13.tar.bz2 8451f5d771edc0a0302bd9a52d54e150 subversion-1.6.13.tar.gz dd4009b239a5354b434e5f66cddde145 subversion-1.6.13.zip 2a7d662bac872c61a5e11c89263d7f07 subversion-deps-1.6.13.tar.bz2 688bdb107731f9db2f3b6297b663a68d subversion-deps-1.6.13.tar.gz bb960d37f835e5e556e23e4eb85a9b08 subversion-deps-1.6.13.zip The SHA1 checksums are: 185efd129c3c4b04f1544d62bb9a3fcd0f58ba29 subversion-1.6.13.tar.bz2 06d3afc49182c80ea712c13409c008d27a4e889b subversion-1.6.13.tar.gz 6530528fae0335cd8495ebf1f2072e2dd9df2e31 subversion-1.6.13.zip e51bffda416a3a9abe068aab6f90d174dedef352 subversion-deps-1.6.13.tar.bz2 1faa5ba0c87210f534ed445d061e4955396524d4 subversion-deps-1.6.13.tar.gz 945ef9f68998aedf320af50d220e47a470684a06 subversion-deps-1.6.13.zip PGP Signatures are available at: http://subversion.tigris.org/downloads/subversion-1.6.13.tar.bz2.asc http://subversion.tigris.org/downloads/subversion-1.6.13.tar.gz.asc http://subversion.tigris.org/downloads/subversion-1.6.13.zip.asc http://subversion.tigris.org/downloads/subversion-deps-1.6.13.tar.bz2.asc http://subversion.tigris.org/downloads/subversion-deps-1.6.13.tar.gz.asc http://subversion.tigris.org/downloads/subversion-deps-1.6.13.zip.asc For this release, the following people have provided PGP signatures: Senthil Kumaran S [1024D/6CCD4038] with fingerprint: 8035 16A5 1D6E 50E2 1ECD DE56 F68D 46FB 6CCD 4038 Philip Martin [2048R/ED1A599C] with fingerprint: A844 790F B574 3606 EE95 9207 76D7 88E1 ED1A 599C Paul T. Burba [1024D/53FCDC55] with fingerprint: E630 CF54 792C F913 B13C 32C5 D916 8930 53FC DC55 Julian Foad [1024D/353E25BC] with fingerprint: 6604 5A4B 43BC F994 5728 351F 33E4 353E 25BC Bert Huijben [1024D/9821F7B2] with fingerprint: 2017 F51A 2572 0E78 8827 5329 FCFD 6305 9821 F7B2 Hyrum K. Wright [1024D/4E24517C] with fingerprint: 3324 80DA 0F8C A37D AEE6 D084 0B03 AE6E 4E24 517C Stefan Sperling [1024D/F59D25F0] with fingerprint: B1CF 1060 A1E9 34D1 9E86 D6D6 E5D3 0273 F59D 25F0 Mark Phippard [1024D/035A96A9] with fingerprint: D315 89DB E1C1 E9BA D218 39FD 265D F8A0 035A 96A9 Release notes for the 1.6.x release series may be found at: http://subversion.apache.org/docs/release-notes/1.6.html You can find the list of changes between 1.6.13 and earlier versions at: http://svn.apache.org/repos/asf/subversion/tags/1.6.13/CHANGES Questions, comments, and bug reports to us...@subversion.apache.org. Thanks, - The Subversion Team
Re: svn commit: r1003585 - in /subversion/site/publish: index.html news.html
On 10/01/2010 10:03 AM, hwri...@apache.org wrote: Author: hwright Date: Fri Oct 1 17:03:11 2010 New Revision: 1003585 URL: http://svn.apache.org/viewvc?rev=1003585view=rev Log: Announce Subversion 1.6.12. You mean 1.6.13? Blair
Re: svn commit: r1003585 - in /subversion/site/publish: index.html news.html
On Fri, Oct 1, 2010 at 12:10 PM, Blair Zajac bl...@orcaware.com wrote: On 10/01/2010 10:03 AM, hwri...@apache.org wrote: Author: hwright Date: Fri Oct 1 17:03:11 2010 New Revision: 1003585 URL: http://svn.apache.org/viewvc?rev=1003585view=rev Log: Announce Subversion 1.6.12. You mean 1.6.13? Yep. Fixed. -Hyrum
Re: 1.6.13 up for signing/testing
The release is now public, and I've published the CVE advisory (and linked to it from the release announcement). Thanks to everybody who tested and signed this release (or just kicked the tires a bit :) ) A very tired, -Hyrum On Wed, Sep 29, 2010 at 3:08 PM, Hyrum K. Wright hyrum_wri...@mail.utexas.edu wrote: 1.6.13 tarballs are up for testing and signing. The magic revision is r1002816: http://people.apache.org/~hwright/svn/1.6.13/ As usual, signatures from full committers please send back to me. Testing by enthusiastic users is also welcomed (but remember that this is not yet a blessed release, with all that that implies). If you are a package maintainer, please do not included this release in your distribution until after it has been formally released. I'd like to collect all the signatures in time to do a release by October 1.
Re: svn commit: r1002898 - in /subversion/trunk: ./ subversion/include/svn_string.h subversion/libsvn_subr/svn_string.c
On Fri, Oct 1, 2010 at 04:36, Stefan Fuhrmann stefanfuhrm...@alice-dsl.de wrote: ... I mean calling convention. Where is the Subversion ABI defined? subversion/include/ It follows guidelines from: http://apr.apache.org/versioning.html With some additional clarifications/notes at: http://subversion.apache.org/docs/community-guide/releasing.html#release-numbering Cheers, -g
Re: trunk failing tests on Windows XP (32 bit): prop-tests.py 33, stat-tests.py 5, upgrade-tests.py 11
On Fri, Oct 1, 2010 at 3:45 PM, Paul Burba ptbu...@gmail.com wrote: On Thu, Sep 30, 2010 at 8:10 PM, Bert Huijben b...@qqmail.nl wrote: -Original Message- From: Johan Corveleyn [mailto:jcor...@gmail.com] Sent: vrijdag 1 oktober 2010 1:51 To: Subversion Development Subject: trunk failing tests on Windows XP (32 bit): prop-tests.py 33, stat-tests.py 5, upgrade-tests.py 11 Hi devs, The following tests fail on my machine (Windows XP 32-bit, built with VCE 2008, ra_local): - prop-tests.py 33: test properties of obstructed subdirectories svn: Can't open directory 'C:\research\svn\client_build\svn_trunk\Release\subversion\tests\cmdlin e\svn-test-work\working_copies\prop_tests-33\A\C': The directory name is invalid. - stat-tests.py 5: status on versioned items whose type has changed svn: Can't open directory 'C:\research\svn\client_build\svn_trunk\Release\subversion\tests\cmdlin e\svn-test-work\working_copies\stat_tests-5\A': The directory name is invalid. - upgrade_tests.py 11: missing directories and obstructing files svn: Can't open directory 'C:\research\svn\client_build\svn_trunk\Release\subversion\tests\cmdlin e\svn-test-work\working_copies\upgrade_tests-11\A\D': The directory name is invalid. They all seem to try to open some path as a directory, but it isn't a directory (but an empty file or something). Am I the only one seeing this? Is this known/normal? This seems to be related to the APR version you use. (Paul Burba reported this same problem earlier this week). Hi Johan, Bert is correct, I saw these failure earlier this week while using APR 1.3.12. All the failures occur when a file unexpectedly obstructs a directory. The APR macro APR_STATUS_IS_ENOTDIR in 1.3.x does not handle the ERROR_DIRECTORY 267 The directory name is invalid that Windows raises in this case. This was fixed in APR in http://svn.apache.org/viewvc?view=revisionrevision=821306 and is in APR 1.4.2. Thanks for the info. I was/am still building against APR 1.3.8, so that explains it. So, will 1.4.2 become the minimum supported APR version for SVN? Or maybe a future 1.3.x release that fixes the same issue? Or will SVN trunk be changed/fixed to cope with this? Since this problem only appeared a week ago, it must be that something in svn code changed to provoke this problem, right? It used to work ... Cheers, -- Johan
