Re: Move contrib/ to apache-extras.org
On 15.12.2010 07:05, Blair Zajac wrote: On 12/14/10 8:28 PM, Hyrum K. Wright wrote: I'd like to move everything in our contrib/ directory to the new Apache Extras hosting. As many might know, I've been on a personal crusade to stop the Subversion project from being a source code hosting system. In the early days, when it was useful for people to have a collection of Subversion-related scripts, contrib/ was valuable, but with search engines and hosting providers, it seems redundant for us to continue hosting contrib/. Because of the variety of licensing situations, we don't even ship it anymore. As one of the people that was opposing your crusade :) , I'm OK with the move if we can host projects with non-Apache licenses. I don't recall reading if that's possible or not? The official announcement said: As projects on the new Google-hosted service will not be managed by The Apache Software Foundation, participants are allowed to use whatever license and project management process they desire. Nevertheless the Apache License is recommended, e.g. it would ease the move into the incubator. I found three short pages with info: http://code.google.com/a/apache-extras.org/p/apache-extras/ http://code.google.com/a/apache-extras.org/p/apache-extras/wiki/FAQ http://community.apache.org/apache-extras/guidelines.html Regards, Rainer
Re: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?)
On 15.12.2010 02:30, Stefan Fuhrmann wrote: On 14.12.2010 23:35, Johan Corveleyn wrote: Some considerations: - Maybe I can use APR_INLINE, with similar results? - Maybe I can put just the critical section into a macro (the curp++ / curp-- part), and call a function when a chunk boundary is encountered (~ once every 131072 iterations (chunks are 128 Kb large)), to read in the new chunk, advancing variables, ... Prefer inlining because the compiler is free to ignore it. Depending on the target architecture and the compiler, it may be beneficial to narrow the scope of optimization: In large functions, the optimizer may guess wrongly about the hot spots. Certainly. These days it makes no sense to use macros instead of inlining, except in really, really extreme cases (of which there should be none in Subversion code), or where macros are used for tricks that the language proper doesn't support (debug-mode error location reporting being such an example). Which raises the question -- did you run your test case with a fully-optimized build, or with some debug-friendly build? Compilers these days /should/ be able to automatically inline static functions, regardless of explicit inline declarations -- Brane
Re: svn commit: r1049414 - /subversion/trunk/subversion/tests/svn_test_main.c
On 15.12.2010 06:17, Blair Zajac wrote: On 12/14/10 8:21 PM, hwri...@apache.org wrote: Author: hwright Date: Wed Dec 15 04:21:23 2010 New Revision: 1049414 URL: http://svn.apache.org/viewvc?rev=1049414view=rev Log: Allow tests which cause segfaults to not disrupt the other tests in the C testsuite. This becomes useful when committing an XFailing test of a segfault (which I will shortly do), or when a test starts segfaulting of it's own accord. This behavior is enabled by default, but can be disabled by passing --allow-segfaults to the test, which should make for better debugging of the cause of the segfault. This patch uses setjmp() / longjmp() to accomplish the recovery from a SIGSEGV. While I'd highly frown upon those function in general purpose use, I believe this is an understandable exception. Because it is so rare to use them, my personal experience is limited, so review and corrections are of course welcomed. I've never seen setjmp/longjump used for this, but it's interesting. One question, could you use sigsetjmp/siglongjmp instead of manually putting back disposition the to default with apr_signal? I guess you would use apr_signal after the sigsetjump in that case. Are the sig* variants available on all platforms? I don't recall seeing them in the C90 stdlib. -- Brane
Re: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?)
On 15.12.2010 02:30, Stefan Fuhrmann wrote:
On 14.12.2010 23:35, Johan Corveleyn wrote:
Thoughts?
Two things you might try:
* introduce a local variable for afile[i]
* replace the for() loop with two nested ones, keeping
calls to other functions out of the hot spot:
for (i=0; i file_len;)
That should read:
for (i=0; i file_len; i++)
{
/* hot spot: */
for(; i file_len; i++)
{
curFile = afile[i];
if (curFile-chunk==-1)
curFile-chunk = 0;
else if (curFile-curp != curFile-endp -1)
curFile-curp++;
else
break;
}
if (i file_len)
{
/* the complex, rarely used stuff goes here */
}
}
-- Stefan^2
Re: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?)
On Wed, Dec 15, 2010 at 10:30 AM, Branko Čibej br...@xbc.nu wrote: On 15.12.2010 02:30, Stefan Fuhrmann wrote: On 14.12.2010 23:35, Johan Corveleyn wrote: Some considerations: - Maybe I can use APR_INLINE, with similar results? - Maybe I can put just the critical section into a macro (the curp++ / curp-- part), and call a function when a chunk boundary is encountered (~ once every 131072 iterations (chunks are 128 Kb large)), to read in the new chunk, advancing variables, ... Prefer inlining because the compiler is free to ignore it. Depending on the target architecture and the compiler, it may be beneficial to narrow the scope of optimization: In large functions, the optimizer may guess wrongly about the hot spots. Certainly. These days it makes no sense to use macros instead of inlining, except in really, really extreme cases (of which there should be none in Subversion code), or where macros are used for tricks that the language proper doesn't support (debug-mode error location reporting being such an example). Which raises the question -- did you run your test case with a fully-optimized build, or with some debug-friendly build? Compilers these days /should/ be able to automatically inline static functions, regardless of explicit inline declarations It was a Windows build (x86), with Release configuration, compiled with Visual Studio Express 2008 (when measuring performance of this branch, I always use a Release build, a Debug build is at least twice as slow; I only change to a Debug build if I'm having problems, needing to debug etc.). I have not used any special compilation options, so maybe that could also make a difference (but, to be honest, I'm lacking the expert knowledge about C compilers, options, platform specific things etc. for this). Maybe the function is a little bit too large/complex for the compiler to automatically inline it without any hints? Or maybe writing it differently, like Stefan suggests, could help the compiler to inline it without needing explicit inline declarations... I'll experiment a little bit (in a couple of days, having a huge shortage of time now :-)). -- Johan
Re: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?)
On Wed, Dec 15, 2010 at 2:30 AM, Stefan Fuhrmann eq...@web.de wrote:
On 14.12.2010 23:35, Johan Corveleyn wrote:
Hi all,
Hi Johan ;)
Hi Stefan, thanks for the input :). I suspected that you might have
some ideas about this ...
On the diff-optimizations-bytes branch, in diff_file.c, there are two
functions which are called for every byte of the identical prefix and
suffix: increment_pointers and decrement_pointers. These functions are
actually equivalents of curp++ or curp--, reading the next/previous
byte, but taking into account the chunked-ness of the data (file data
is read in memory in chunks).
As an experiment I changed these functions into macro's, eliminating
the function calls. This makes the diff algorithm another 10% - 15%
faster (granted, this was measured with my extreme testcase of a 1,5
Mb file (6 lines), of which most lines are identical
prefix/suffix). However, having an entire function like that
implemented as a macro isn't very pretty (see below for an example).
I'm kind of surprised that the calling overhead is
actually noticeable for larger files, i.e. larger values
of file_len it should loop many times.
file_len is the size of the array of files, not the length of the
files themselves. In the typical case (the only case that's currently
supported) file_len is 2. That's because we're diffing just 2 files:
the original one and the modified one. The implementation is made
with an array and a file_len (and for loops), because I wanted it to
be generically useful also for diff3 (with 3 files: original, modified
and latest) and diff4 (4 files: original, modified, latest and
ancestor).
Also, I did my measurements with a blame operation on this very large
file, which has ~2500 revisions. So that's 2500 diffs of a ~1,5 Mb
file, with say on average 1 Mb of identical prefix/suffix every time.
That's some 2,500,000,000 function calls.
For measurement, I counted the total time of the blame operation, as
well as the cumulative time for the svn_diff_diff calls (with
apr_time_now() before and after).
Some considerations:
- Maybe I can use APR_INLINE, with similar results?
- Maybe I can put just the critical section into a macro (the curp++
/ curp-- part), and call a function when a chunk boundary is
encountered (~ once every 131072 iterations (chunks are 128 Kb
large)), to read in the new chunk, advancing variables, ...
Prefer inlining because the compiler is free to ignore it.
Depending on the target architecture and the compiler,
it may be beneficial to narrow the scope of optimization:
In large functions, the optimizer may guess wrongly about
the hot spots.
- Maybe it's not worth it?
Since inlining is for free from the maintenance POV,
any gain should justify it.
Thoughts?
Two things you might try:
* introduce a local variable for afile[i]
* replace the for() loop with two nested ones, keeping
calls to other functions out of the hot spot:
for (i=0; i file_len;)
{
/* hot spot: */
for(; i file_len; i++)
{
curFile = afile[i];
if (curFile-chunk==-1)
curFile-chunk = 0;
else if (curFile-curp != curFile-endp -1)
curFile-curp++;
else
break;
}
if (i file_len)
{
/* the complex, rarely used stuff goes here */
}
}
Ok, when I have some time, I will experiment a little bit with
inline, and see if the compiler gets it (I'll try your nested for
loop example (the corrected one, which you just sent in the other
mail) to help the compiler a little bit). I should be able to easily
compare that with the macro version.
--
Johan
Re: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?)
On Wed, Dec 15, 2010 at 11:53 AM, Johan Corveleyn jcor...@gmail.com wrote: On Wed, Dec 15, 2010 at 11:50 AM, Johan Corveleyn jcor...@gmail.com wrote: Also, I did my measurements with a blame operation on this very large file, which has ~2500 revisions. So that's 2500 diffs of a ~1,5 Mb file, with say on average 1 Mb of identical prefix/suffix every time. That's some 2,500,000,000 function calls. That should be: 5,000,000,000 function calls, because for every diff we advance prefix and suffix pointers in two files: the original one and the modified one. Damn. Scratch that last mail. I'm confused. It's 2,500,000,000 function calls (each of which advances the two pointers in a for loop). (that's sleep-deprivation in action (sick child etc ...)) -- Johan
RE: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?)
-Original Message- From: Johan Corveleyn [mailto:jcor...@gmail.com] Sent: woensdag 15 december 2010 11:30 To: Branko Čibej Cc: dev@subversion.apache.org Subject: Re: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?) It was a Windows build (x86), with Release configuration, compiled with Visual Studio Express 2008 (when measuring performance of this branch, I always use a Release build, a Debug build is at least twice as slow; I only change to a Debug build if I'm having problems, needing to debug etc.). Not really useful in this discussion, but Visual Studio Express 2008 doesn't have the optimizing C++ compiler that is available in the higher Visual Studio versions. In debug builds on VC++ some additional checks are enabled which will slow things down, but the optimizing compiler (which *is* available in the free to download platform SDK) will improve performance more than just disabling of the debug checks. Bert
Re: [PATCH]: transform_libtool_scripts.py
Here is version 2 of the patch.
On Tue, 14 Dec 2010, Philip Martin wrote:
Martin Furter m...@rola.ch writes:
The generated script then has to be included into the tarball. I
believe to remember that there's a script which creates the
tarball. Can anyone give me a pointer where I can find it?
tools/dist/dist.sh
The tarball is created after autogen.sh has been run; autogen.sh runs
gen-make.py.
No changes seem to be needed there since it's run in the sandbox.
- Martin
[[[
Generate build/transform_libtool_scripts.sh from gen-make.py using
information read from build.conf.
* build/generator/gen_make.py
(write_transform_libtool_scripts,_get_all_lib_deps):
New functions which generate transform_libtool_scripts.sh.
(write): Call write_transform_libtool_scripts.
* build/transform_libtool_scripts.sh
Delete.
]]]Index: build/transform_libtool_scripts.sh
===
--- build/transform_libtool_scripts.sh (revision 1049476)
+++ build/transform_libtool_scripts.sh (working copy)
@@ -1,123 +0,0 @@
-#!/bin/sh
-#
-#
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements. See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership. The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# License); you may not use this file except in compliance
-# with the License. You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied. See the License for the
-# specific language governing permissions and limitations
-# under the License.
-#
-#
-
-# Dependencies of libraries
-# TODO: generate from build.conf
-subr=subr
-auth_gnome_keyring=auth_gnome_keyring $subr
-auth_kwallet=auth_kwallet $subr
-delta=delta $subr
-diff=diff $subr
-fs_util=fs_util $subr
-fs_base=fs_base $delta $fs_util $subr
-fs_fs=fs_fs $delta $fs_util $subr
-fs=fs $fs_base $fs_fs $fs_util $subr
-repos=repos $delta $fs $fs_util $subr
-ra_local=ra_local $delta $fs $fs_util $repos $subr
-ra_neon=ra_neon $delta $subr
-ra_serf=ra_serf $delta $subr
-ra_svn=ra_svn $delta $subr
-ra=ra $delta $ra_local $ra_neon $ra_serf $ra_svn $subr
-wc=wc $delta $diff $subr
-client=client $delta $diff $ra $subr $wc
-
-# Delete duplicates in dependencies of libraries
-ls subversion | grep libsvn_ | while read library_dir; do
- library=`basename $library_dir | sed s/libsvn_//`
- library_dependencies=$(echo -n $(for x in $(eval echo \$$library); do
echo $x; done | sort -u))
- eval $library=\$library_dependencies
-done
-
-# Dependencies of executables
-svn=$auth_gnome_keyring $auth_kwallet $client $delta $diff $ra $subr $wc
-svnadmin=$delta $fs $repos $subr
-svndumpfilter=$delta $fs $repos $subr
-svnlook=$delta $diff $fs $repos $subr
-svnserve=$delta $fs $ra_svn $repos $subr
-svnsync=$auth_gnome_keyring $auth_kwallet $delta $ra $subr
-svnversion=$subr $wc
-entries_dump=$subr $wc
-atomic_ra_revprop_change=$subr $ra
-
-# Variable 'executables' containing names of variables corresponding to
executables
-executables=svn svnadmin svndumpfilter svnlook svnserve svnsync svnversion
entries_dump atomic_ra_revprop_change
-
-for executable in $executables; do
- # Set variables containing paths of executables
- eval ${executable}_path=subversion/$executable/$executable
- if [ $executable = entries_dump ]; then
-eval ${executable}_path=subversion/tests/cmdline/entries-dump
- fi
- if [ $executable = atomic_ra_revprop_change ]; then
-eval ${executable}_path=subversion/tests/cmdline/atomic-ra-revprop-change
- fi
- # Delete duplicates in dependencies of executables
- executable_dependencies=$(echo -n $(for x in $(eval echo \$$executable);
do echo $x; done | sort -u))
- eval $executable=\$executable_dependencies
-done
-
-test_paths=$(find subversion/tests -mindepth 2 -maxdepth 2 -name '*-test' !
-path '*/.libs/*' | sort)
-for test in $test_paths; do
- test_path=$test
- # Dependencies of tests are based on names of directories containing tests
- test_library=$(echo $test | sed -e
's:^subversion/tests/libsvn_\([^/]*\)/.*:\1:')
- test_dependencies=$(eval echo \$$test_library)
- # Set variables corresponding to tests and containing dependencies of tests
- test=$(echo $test | sed -e 's:^subversion/tests/libsvn_[^/]*/\(.*\):\1:' -e
's/-/_/g')
- eval $test=\$test_dependencies
- # Set variables containing paths of tests
- eval ${test}_path=\$test_path
- # Set variable 'tests' containing names of variables corresponding to tests
- tests=$tests $test
-done
-
-# auth-test dynamically loads libsvn_auth_gnome_keyring and
libsvn_auth_kwallet libraries
-auth_test=auth_gnome_keyring auth_kwallet $auth_test
-
-# Usage: sed_append
Re: svn commit: r1049414 - /subversion/trunk/subversion/tests/svn_test_main.c
On Wed, Dec 15, 2010 at 3:32 AM, Branko Čibej br...@apache.org wrote: On 15.12.2010 06:17, Blair Zajac wrote: On 12/14/10 8:21 PM, hwri...@apache.org wrote: Author: hwright Date: Wed Dec 15 04:21:23 2010 New Revision: 1049414 URL: http://svn.apache.org/viewvc?rev=1049414view=rev Log: Allow tests which cause segfaults to not disrupt the other tests in the C testsuite. This becomes useful when committing an XFailing test of a segfault (which I will shortly do), or when a test starts segfaulting of it's own accord. This behavior is enabled by default, but can be disabled by passing --allow-segfaults to the test, which should make for better debugging of the cause of the segfault. This patch uses setjmp() / longjmp() to accomplish the recovery from a SIGSEGV. While I'd highly frown upon those function in general purpose use, I believe this is an understandable exception. Because it is so rare to use them, my personal experience is limited, so review and corrections are of course welcomed. I've never seen setjmp/longjump used for this, but it's interesting. One question, could you use sigsetjmp/siglongjmp instead of manually putting back disposition the to default with apr_signal? I guess you would use apr_signal after the sigsetjump in that case. Are the sig* variants available on all platforms? I don't recall seeing them in the C90 stdlib. From my manpage: The setjmp() and longjmp() functions conform to ISO/IEC 9899:1990 (``ISO C90''). The sigsetjmp() and siglongjmp() functions conform to IEEE Std 1003.1-1988 (``POSIX.1''). So it would appear that the sig* variants are not part of the C90 standard. -Hyrum
Re: svn commit: r1049414 - /subversion/trunk/subversion/tests/svn_test_main.c
On 12/15/2010 04:32 AM, Branko Čibej wrote: Are the sig* variants available on all platforms? I don't recall seeing them in the C90 stdlib. `man setjmp` tells me: C89, C99, and POSIX.1-2001 specify setjmp(). POSIX.1-2001 specifies sigsetjmp(). -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: Move contrib/ to apache-extras.org
On Wed, Dec 15, 2010 at 8:02 AM, C. Michael Pilato cmpil...@collab.net wrote: On 12/14/2010 11:28 PM, Hyrum K. Wright wrote: I'd like to move everything in our contrib/ directory to the new Apache Extras hosting. +`find trunk/contrib | wc -l`. $ find contrib | wc -l 592 $ find contrib | grep -v '.svn' | wc -l 36 :) -Hyrum
[PATCH] Remove redundant member - libsvn_wc/update_editor.c:file_baton.added_with_history
Hi,
I was looking through the subversion/libsvn_wc/update_editor.c and
noticed that there was a redundant member (added_with_history) in the
file baton structure. This is not being set anywhere which results in
some unreachable code. I have removed the member and the corresponding
unreachable code. This was (as I gather from svn blame) introduced in
r867451 by glasser to implement copy-on-update. I understand that some
of this was reverted by cmpilato. I guess this should have been removed
too. I have attached a patch and a log message. Please review and respond.
Regards,
Arwin Arni
Index: subversion/libsvn_wc/update_editor.c
===
--- subversion/libsvn_wc/update_editor.c(revision 1049559)
+++ subversion/libsvn_wc/update_editor.c(working copy)
@@ -908,9 +908,6 @@
/* Set if this file is new. */
svn_boolean_t adding_file;
- /* Set if this file is new with history. */
- svn_boolean_t added_with_history;
-
/* Set if an unversioned file of the same name already existed in
this directory. */
svn_boolean_t obstruction_found;
@@ -3859,7 +3856,7 @@
svn_node_kind_t wfile_kind;
SVN_ERR(svn_io_check_path(fb-local_abspath, wfile_kind, pool));
- if (wfile_kind == svn_node_none ! fb-added_with_history)
+ if (wfile_kind == svn_node_none)
{
/* working file is missing?!
Just copy the new text-base to the file. */
@@ -3890,14 +3887,6 @@
path_ext = ;
}
- /* Create strings representing the revisions of the
- old and new text-bases. */
- /* Either an old version, or an add-with-history */
- if (fb-added_with_history)
-oldrev_str = apr_psprintf(pool, .copied%s%s,
- *path_ext ? . : ,
- *path_ext ? path_ext : );
- else
{
svn_revnum_t old_rev = revision;
@@ -4137,14 +4126,6 @@
SVN_ERR_ASSERT(new_text_base_md5_checksum
new_text_base_sha1_checksum);
}
- else if (fb-added_with_history)
-{
- SVN_ERR_ASSERT(! fb-new_text_base_sha1_checksum);
- new_text_base_md5_checksum = fb-copied_text_base_md5_checksum;
- new_text_base_sha1_checksum = fb-copied_text_base_sha1_checksum;
- SVN_ERR_ASSERT(new_text_base_md5_checksum
- new_text_base_sha1_checksum);
-}
else
{
SVN_ERR_ASSERT(! fb-new_text_base_sha1_checksum
[[[
Removed a redundant member (added_with_history) from the file baton
and removed *relevent* unreachable code.
* subversion/libsvn_wc/update_editor.c
(struct file_baton): Removed 'added_with_history'.
(merge_file, close_file): Removed unreachable code.
Patch by: Arwin Arni arwin_at_collab.net
]]]
Re: [Issue 3758] New - svn-dev.el - Update paths and links
C. Michael Pilato cmpil...@collab.net writes: On 12/10/2010 11:31 AM, Noorul Islam K M wrote: Noorul Islam K M noo...@collab.net writes: Attached is the patch. FIXED: Sendingtools/dev/svn-dev.el Transmitting file data . Committed revision 1049557. Thank You! - Noorul
stat_tests.py 19 is failing?
I'm seeing stat_tests.py 19 failing for what looks like a difference of abspath/relpath in the output: -svn: warning: '/home/cmpilato/projects/subversion/trunk/subversion/tests/cmdline/svn-test-work/local_tmp/repos' is not a working copy -svn: warning: '/home/cmpilato/projects/subversion/trunk/subversion/tests/cmdline/svn-test-work/local_tmp/repos' is not a working copy +svn: warning: 'svn-test-work/local_tmp/repos' is not a working copy +svn: warning: 'svn-test-work/local_tmp/repos' is not a working copy My work-in-progress *shouldn't* have any relevance here, so I'm assuming that others are seeing this problem, too. Can someone confirm? -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: stat_tests.py 19 is failing?
C. Michael Pilato cmpil...@collab.net writes: I'm seeing stat_tests.py 19 failing for what looks like a difference of abspath/relpath in the output: -svn: warning: '/home/cmpilato/projects/subversion/trunk/subversion/tests/cmdline/svn-test-work/local_tmp/repos' is not a working copy -svn: warning: '/home/cmpilato/projects/subversion/trunk/subversion/tests/cmdline/svn-test-work/local_tmp/repos' is not a working copy +svn: warning: 'svn-test-work/local_tmp/repos' is not a working copy +svn: warning: 'svn-test-work/local_tmp/repos' is not a working copy My work-in-progress *shouldn't* have any relevance here, so I'm assuming that others are seeing this problem, too. Can someone confirm? I confirm that it is failing for me also. I do not have any local changes. Thanks and Regards Noorul
Re: stat_tests.py 19 is failing?
C. Michael Pilato cmpil...@collab.net writes: I'm seeing stat_tests.py 19 failing for what looks like a difference of abspath/relpath in the output: stat_tests.py 19 is a PASS for me, unmodified r11049592. -- Philip
Re: stat_tests.py 19 is failing?
Philip Martin philip.mar...@wandisco.com writes: C. Michael Pilato cmpil...@collab.net writes: I'm seeing stat_tests.py 19 failing for what looks like a difference of abspath/relpath in the output: stat_tests.py 19 is a PASS for me, unmodified r11049592. s/r11049578/r1049592/ -- Philip
Re: stat_tests.py 19 is failing?
Philip Martin phi...@codematters.co.uk writes: stat_tests.py 19 is a PASS for me, unmodified r11049592. s/r11049578/r1049592/ Gah! I mean r1049592. -- Philip
Re: Subversion issue 2791
Here's the latest patch. I ran the commit tests and svnlook tests and they
passed. There seem to not be any other tests for hooks.
- Martin
[[[
Use svn_io_start_cmd2 in run_hook_cmd.
* subversion/libsvn_repos/hooks.c
(run_hook_cmd): Use svn_io_start_cmd2.
]]]Index: subversion/libsvn_repos/hooks.c
===
--- subversion/libsvn_repos/hooks.c (revision 1049476)
+++ subversion/libsvn_repos/hooks.c (working copy)
@@ -177,56 +177,14 @@
apr_file_t *stdin_handle,
apr_pool_t *pool)
{
- apr_file_t *read_errhandle, *write_errhandle, *null_handle;
- apr_file_t *read_outhandle, *write_outhandle;
+ apr_file_t *null_handle;
apr_status_t apr_err;
svn_error_t *err;
apr_proc_t cmd_proc;
- /* Create a pipe to access stderr of the child. */
- apr_err = apr_file_pipe_create(read_errhandle, write_errhandle, pool);
- if (apr_err)
-return svn_error_wrap_apr
- (apr_err, _(Can't create pipe for hook '%s'), cmd);
-
- /* Pipes are inherited by default, but we don't want that, since
- APR will duplicate the write end of the pipe for the child process.
- Not closing the read end is harmless, but if the write end is inherited,
- it will be inherited by grandchildren as well. This causes problems
- if a hook script puts long-running jobs in the background. Even if
- they redirect stderr to something else, the write end of our pipe will
- still be open, causing us to block. */
- apr_err = apr_file_inherit_unset(read_errhandle);
- if (apr_err)
-return svn_error_wrap_apr
- (apr_err, _(Can't make pipe read handle non-inherited for hook '%s'),
- cmd);
-
- apr_err = apr_file_inherit_unset(write_errhandle);
- if (apr_err)
-return svn_error_wrap_apr
- (apr_err, _(Can't make pipe write handle non-inherited for hook '%s'),
- cmd);
-
if (result)
{
- /* Create a pipe to access stdout of the child. */
- apr_err = apr_file_pipe_create(read_outhandle, write_outhandle, pool);
- if (apr_err)
-return svn_error_wrap_apr
- (apr_err, _(Can't create pipe for hook '%s'), cmd);
-
- apr_err = apr_file_inherit_unset(read_outhandle);
- if (apr_err)
-return svn_error_wrap_apr
- (apr_err,
- _(Can't make pipe read handle non-inherited for hook '%s'), cmd);
-
- apr_err = apr_file_inherit_unset(write_outhandle);
- if (apr_err)
-return svn_error_wrap_apr
- (apr_err,
- _(Can't make pipe write handle non-inherited for hook '%s'), cmd);
+ null_handle = NULL;
}
else
{
@@ -238,27 +196,10 @@
(apr_err, _(Can't create null stdout for hook '%s'), cmd);
}
- err = svn_io_start_cmd(cmd_proc, ., cmd, args, FALSE,
- stdin_handle, result ? write_outhandle : null_handle,
- write_errhandle, pool);
+ err = svn_io_start_cmd2(cmd_proc, ., cmd, args, FALSE,
+ FALSE, stdin_handle, result != NULL, null_handle,
+ TRUE, NULL, pool);
- /* This seems to be done automatically if we pass the third parameter of
- apr_procattr_child_in/out_set(), but svn_io_run_cmd()'s interface does
- not support those parameters. We need to close the write end of the
- pipe so we don't hang on the read end later, if we need to read it. */
- apr_err = apr_file_close(write_errhandle);
- if (!err apr_err)
-return svn_error_wrap_apr
- (apr_err, _(Error closing write end of stderr pipe));
-
- if (result)
-{
- apr_err = apr_file_close(write_outhandle);
- if (!err apr_err)
-return svn_error_wrap_apr
- (apr_err, _(Error closing write end of stderr pipe));
-}
-
if (err)
{
err = svn_error_createf
@@ -266,13 +207,13 @@
}
else
{
- err = check_hook_result(name, cmd, cmd_proc, read_errhandle, pool);
+ err = check_hook_result(name, cmd, cmd_proc, cmd_proc.err, pool);
}
/* Hooks are fallible, and so hook failure is expected to occur at
times. When such a failure happens we still want to close the pipe
and null file */
- apr_err = apr_file_close(read_errhandle);
+ apr_err = apr_file_close(cmd_proc.err);
if (!err apr_err)
return svn_error_wrap_apr
(apr_err, _(Error closing read end of stderr pipe));
@@ -280,8 +221,8 @@
if (result)
{
svn_stringbuf_t *native_stdout;
- SVN_ERR(svn_stringbuf_from_aprfile(native_stdout, read_outhandle,
pool));
- apr_err = apr_file_close(read_outhandle);
+ SVN_ERR(svn_stringbuf_from_aprfile(native_stdout, cmd_proc.out, pool));
+ apr_err = apr_file_close(cmd_proc.out);
if (!err apr_err)
return svn_error_wrap_apr
(apr_err, _(Error closing read end of stderr pipe));
Re: [PATCH]: transform_libtool_scripts.py
Martin Furter m...@rola.ch writes: Doing some more testing I found another out that non-existing libraries have to be removed from LD_PRELOAD. Does this interact with --enable-runtime-module-search? That's the switch that causes Subversion to load RA/FS modules at runtime rather than linking them to the binary. In the past it was difficult to run the regression tests with this enabled because the runtime loader would pick installed modules ahead of the ones in the build directory. -- Philip
Re: stat_tests.py 19 is failing?
On 12/15/2010 02:02 PM, Philip Martin wrote: If I do pushd subversion/tests/cmdline svnadmin create repo ../../svn/svn co file://`pwd`/repo wc mv wc/.svn . popd then stat_tests.py 19 starts to FAIL. So it's running inside a 1.7 wc that causes the problem. Try r1049668 -- see if stuff still passes for you in your default configuration. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: stat_tests.py 19 is failing?
On 12/15/2010 02:11 PM, Philip Martin wrote: C. Michael Pilato cmpil...@collab.net writes: Try r1049668 -- see if stuff still passes for you in your default configuration. Yes, it does. Cool. Thanks. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: [PATCH] svnrdump help displays subcommand-specific documentation
On 12/15/2010 08:37 AM, Prabhu Gnana Sundar wrote: Hi, This patch makes the svnrdump's help to display the subcommand-specific documentation. ( similar to svn, svnlook, svnadmin ...) Previously, svnrdump dump --help does *not* display the help-documentation corresponding to dump, but svnrdump help dump did as expected. Meanwhile, few other observations in the *current* behaviour are : 1. The svnrdump does *not* display the generic documentation as follows... Type 'svn --version' to see the program version and RA modules eventhough it supports svnrdump --version. 2. svnrdump --version --junk also displays the version documentation, which it should *not*. I am working on these observations to make them behave uniformly and will come up in further patches. I have attached the patch and the log file with this mail. Committed: Sendingtrunk/subversion/svnrdump/svnrdump.c Transmitting file data . Committed revision 1049672. Thanks, Prabhu. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: stat_tests.py 19 is failing?
On 12/15/2010 03:00 PM, C. Michael Pilato wrote: On 12/15/2010 02:58 PM, Philip Martin wrote: C. Michael Pilato cmpil...@collab.net writes: On 12/15/2010 02:11 PM, Philip Martin wrote: C. Michael Pilato cmpil...@collab.net writes: Try r1049668 -- see if stuff still passes for you in your default configuration. Yes, it does. Cool. Thanks. It seems to fail on Windows: http://ci.apache.org/builders/svn-slik-w2k3-x64-local/builds/2437 Doh! I wonder if this would do the trick: Index: subversion/tests/cmdline/stat_tests.py === --- subversion/tests/cmdline/stat_tests.py(revision 1049668) +++ subversion/tests/cmdline/stat_tests.py(working copy) @@ -964,7 +964,7 @@ status on unversioned dir (issue 2030) sbox.build(read_only = True) dir = sbox.repo_dir - expected_err = svn: warning: '.*/ + os.path.basename(dir) + \ + expected_err = svn: warning: '.* + os.sep + os.path.basename(dir) + \ ' is not a working copy svntest.actions.run_and_verify_svn2(None, [], expected_err, 0, status, dir, dir) Or would that backslash need to be escaped? *grumble*... -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
