Re: svn commit: r1035894 - undefined reference to 'svn_fs_validate_mergeinfo'
Since r1035894 I'm getting a link error on Linux (and so is Noorul): cd subversion/libsvn_fs_fs /usr/share/apr-1.0/build/libtool --tag=CC --silent --mode=link gcc -W[...] -D[...] -g -pthread -rpath /home/julianfoad/local/subversion-d/lib -Wl,--no-undefined -o libsvn_fs_fs-1.la caching.lo dag.lo fs.lo fs_fs.lo id.lo key-gen.lo lock.lo rep-cache.lo tree.lo ../../subversion/libsvn_delta/libsvn_delta-1.la ../../subversion/libsvn_subr/libsvn_subr-1.la -laprutil-1 -lapr-1 ../../subversion/libsvn_fs_util/libsvn_fs_util-1.la .libs/tree.o: In function `get_mergeinfo_for_path': /home/julianfoad/src/subversion-d/subversion/libsvn_fs_fs/tree.c:3595: undefined reference to `svn_fs_validate_mergeinfo' I have deleted my ~/local/subversion-d directory, so it doesn't exist at this stage, and then I have run autogen.sh and configure. It appears that the new function 'svn_fs_validate_mergeinfo' is the only function outside libsvn_fs_fs and libsvn_fs_util that is called directly from within. I don't see why that shouldn't be OK, but it is suspicious. I wonder if the compiler option '-Wl,--no-undefined' is now wrong for this lib. How is that option controlled? Globally, by the configure flag '--enable-disallowing-of-undefined-references' (ugh), but how per lib? Or should this function be moved to libsvn_fs_util/util.c, with merely a public wrapper placed in libsvn_fs/loader.c if required? - Julian On Wed, 2010-11-17, pbu...@apache.org wrote: Log: Reintegrate the issue-3668-3669 branch.
Re: scan_deletion - why?
On Tue, 2010-11-16, Greg Stein wrote: I'm not sure that I understand the goal here. Is there a problem that is trying to be solved? If not, then I'd recommend just marking this down as an item to review in 1.8. Quting myself: My goals: to be able to understand the callers, and then adapt them to op-depth semantics and also simplify them. To expand: I may not be the brightest kid on the block but I find that with hours of effort I still can't see precisely what many of these functions are *trying* to do. (Obviously I can see precisely what they *are* doing in terms of shuffling bits around.) I am extremely frustrated by my slow progress in extending WC code. Something must be wrong: the desired behaviour is complex but not *that* complex. So I am seeking ways to make it more understandable and easier to work with. Basically ways to tie the embedded concepts more obviously and directly to the function names, inputs and outputs. To help myself, and because I am certain it will help others too. If that's too hand-wavey, any of the uses enumerated as 2, 4, 5, 7, 8 below are good concrete examples of the phenomenon. - Julian Cheers, -g On Tue, Nov 16, 2010 at 12:51, Julian Foad julian.f...@wandisco.com wrote: I'm auditing the uses of scan_deletion() and its semi-public wrapper svn_wc__db_scan_deletion(): what do its callers really want? My goals: to be able to understand the callers, and then adapt them to op-depth semantics and also simplify them. It seems to me there's far too much obfuscation going on in these callers at present, with lots of near-but-not-quite-duplication. Here are all the uses and what I can understand about them: 1. adm_crawler.c:find_base_rev() - work_del_abspath This code path is unused in a test suite run. 2. entries.c:get_base_info_for_deleted() (first occurrence) - work_del_abspath Use of result: read_info(dirname(work_del_abspath))+scan_add - repo+relpath Ultimate need: Copyfrom repo+relpath of the deleted node. (The repo+relpath that this path belonged to before it was deleted; if this deletion is inside a move/copy, then the repo location within the copy source.) 3. entries.c:get_base_info_for_deleted() (second occurrence) - base_replaced, work_del_abspath Use of result: base_replaced, read_info(dirname(work_del_abspath))+scan_add - status Ultimate need: Status indications for constructing an entry_t. 4. node.c:svn_wc__node_get_working_rev_info() - base_del_abspath, work_del_abspath This function is only used for back-compat in status reporting. 5. node.c:svn_wc__node_get_commit_base_rev() - work_del_abspath Use of result: read_info(dirname(work_del_abspath))+scan_add - original_rev assert(status is some kind of 'added') Ultimate need: The commit base rev of the deleted node, which probably really means the repository revision of the parent dir of the deletion root. See doc string - should go away. 6. node.c:svn_wc__internal_node_get_schedule() - work_del_abspath Use of result: work_del_abspath == NULL? Ultimate need: To know whether this deletion is inside a copy/move. 7. wc_db.c:get_info_for_copy() - work_del_relpath Use of result: scan_add(dirname(work_del_relpath)) - original_repos_*, op_root Ultimate aim: Copyfrom repos+relpath+rev of the deleted node. ### Relpath of this node, or of root of the copy? 8. wc_db.c:svn_wc__db_global_relocate() - work_del_relpath Use of result: dirname(work_del_relpath) Ultimate aim: Repos+relpath and local path of the parent of the root of the deletion. 9. db-test.c:test_scan_deletion() Several calls, each checking all the output parameters. So what to do with this? Work out what semantics the callers really want, and provide them through one or more APIs. In each case, return the wanted data in a single call instead of providing a path. Then we'll be in a position where we can optimize the queries inside each such function if we need to. 1. Seems unused, so ignore for now. I'm not confident that it can't be triggered. I hope this whole private function can be removed and some (semi-public) API used instead. 2, 5, 7, 8. These are all similar and want to know the repos location of where the deleted node would have been. Need to check carefully what relpath these want: the relpath of the given path, or of the deletion root, or of the parent of the deletion root. Also the revnum - should it be the revnum that the node was at before it was deleted, or the revnum that its parent is at? They can differ in a mixed-rev base or in a copy of a mixed-rev base. 3. Wants status of parent of deletion root. 4. The function for back-compatibility in status reporting. The old, deprecated version of the public status API should retrieve the back-compat fields
Re: svn commit: r1035894 - in /subversion/trunk: ./ subversion/include/ subversion/include/private/ subversion/libsvn_client/ subversion/libsvn_fs/ subversion/libsvn_fs_base/ subversion/libsvn_fs_fs/
Hi Paul. On Wed, 2010-11-17, pbu...@apache.org wrote: Log: Reintegrate the issue-3668-3669 branch. If you could insert a summary of this change here in the log message, that would be helpful. I see it's ... rather large. Modified: subversion/trunk/ (props changed) subversion/trunk/subversion/include/private/svn_dav_protocol.h subversion/trunk/subversion/include/svn_fs.h subversion/trunk/subversion/include/svn_ra.h subversion/trunk/subversion/include/svn_repos.h subversion/trunk/subversion/libsvn_client/copy.c subversion/trunk/subversion/libsvn_client/merge.c subversion/trunk/subversion/libsvn_client/mergeinfo.c subversion/trunk/subversion/libsvn_client/mergeinfo.h subversion/trunk/subversion/libsvn_fs/fs-loader.c subversion/trunk/subversion/libsvn_fs/fs-loader.h subversion/trunk/subversion/libsvn_fs_base/tree.c subversion/trunk/subversion/libsvn_fs_fs/tree.c subversion/trunk/subversion/libsvn_ra/deprecated.c subversion/trunk/subversion/libsvn_ra/ra_loader.c subversion/trunk/subversion/libsvn_ra/ra_loader.h subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c subversion/trunk/subversion/libsvn_ra_neon/mergeinfo.c subversion/trunk/subversion/libsvn_ra_neon/options.c subversion/trunk/subversion/libsvn_ra_neon/ra_neon.h subversion/trunk/subversion/libsvn_ra_serf/mergeinfo.c subversion/trunk/subversion/libsvn_ra_serf/options.c subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h subversion/trunk/subversion/libsvn_ra_svn/client.c subversion/trunk/subversion/libsvn_repos/fs-wrap.c subversion/trunk/subversion/mod_dav_svn/reports/mergeinfo.c subversion/trunk/subversion/svnserve/serve.c subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py subversion/trunk/subversion/tests/cmdline/merge_tests.py
Re: scan_deletion - why?
Julian Foad julian.f...@wandisco.com writes: To expand: I may not be the brightest kid on the block but I find that with hours of effort I still can't see precisely what many of these functions are *trying* to do. (Obviously I can see precisely what they *are* doing in terms of shuffling bits around.) I am extremely frustrated by my slow progress in extending WC code. Something must be wrong: the desired behaviour is complex but not *that* complex. So I am seeking ways to make it more understandable and easier to work with. Basically ways to tie the embedded concepts more obviously and directly to the function names, inputs and outputs. To help myself, and because I am certain it will help others too. We recently decided to stop eliding repos_path/revision in the base node, I think we should stop eliding it from working nodes as well. Then read_info would return it when available, and NULL would be a definitive answer. This might make some of the scan calls redundant. For example, relocate could simply skip nodes that don't have repos_path/revision. -- Philip
The new warning: The depth of this commit is 'empty', but copied dirs ...
Today I committed the result of a merge, which included mergeinfo change on ., but I didn't want to include a file that I have locally modified for testing purposes, so I explicitly specified . and the required target file. [[[ svn commit -N -m ... . subversion/mod_dav_svn/mirror.c svn: warning: The depth of this commit is 'empty', but copied directories will regardless be committed with depth 'infinity'. You must remove unwanted children of those directories in a separate commit. Sending. Sendingsubversion/mod_dav_svn/mirror.c Transmitting file data . Committed revision 1035992. ]]] I think it would be better if we only issue this warning when the proposed commit does in fact include a copy, otherwise it's going to be too much noise. Thoughts? - Julian
Re: The new warning: The depth of this commit is 'empty', but copied dirs ...
On Wed, Nov 17, 2010 at 12:04:21PM +, Julian Foad wrote: Today I committed the result of a merge, which included mergeinfo change on ., but I didn't want to include a file that I have locally modified for testing purposes, so I explicitly specified . and the required target file. [[[ svn commit -N -m ... . subversion/mod_dav_svn/mirror.c svn: warning: The depth of this commit is 'empty', but copied directories will regardless be committed with depth 'infinity'. You must remove unwanted children of those directories in a separate commit. Sending. Sendingsubversion/mod_dav_svn/mirror.c Transmitting file data . Committed revision 1035992. ]]] I think it would be better if we only issue this warning when the proposed commit does in fact include a copy, otherwise it's going to be too much noise. Thoughts? The warning is printed very early in the commit process, way before the commit targets are harvested. Printing the warning at a later stage is much harder to do, especially in light of backporting the change to 1.6.x because that would require API changes (the backport has happened in 1.6.13). But yes, I agree that it would be better to print the warning only if the commit harvester has actually picked up a copy. We could file an issue with 1.7.0 milestone for this, and I'll try to take a crack at it if I can find the time. Stefan
Re: svn commit: r1035894 - undefined reference to 'svn_fs_validate_mergeinfo'
Julian Foad wrote on Wed, Nov 17, 2010 at 10:18:05 +:
Since r1035894 I'm getting a link error on Linux (and so is Noorul):
cd subversion/libsvn_fs_fs /usr/share/apr-1.0/build/libtool --tag=CC
--silent --mode=link gcc -W[...] -D[...] -g -pthread
-rpath /home/julianfoad/local/subversion-d/lib -Wl,--no-undefined -o
libsvn_fs_fs-1.la caching.lo dag.lo fs.lo fs_fs.lo id.lo key-gen.lo
lock.lo rep-cache.lo
tree.lo ../../subversion/libsvn_delta/libsvn_delta-1.la
../../subversion/libsvn_subr/libsvn_subr-1.la -laprutil-1 -lapr-1
../../subversion/libsvn_fs_util/libsvn_fs_util-1.la
.libs/tree.o: In function `get_mergeinfo_for_path':
/home/julianfoad/src/subversion-d/subversion/libsvn_fs_fs/tree.c:3595:
undefined reference to `svn_fs_validate_mergeinfo'
I have deleted my ~/local/subversion-d directory, so it doesn't exist at
this stage, and then I have run autogen.sh and configure.
It appears that the new function 'svn_fs_validate_mergeinfo' is the only
function outside libsvn_fs_fs and libsvn_fs_util that is called directly
from within. I don't see why that shouldn't be OK, but it is
suspicious.
I wonder if the compiler option '-Wl,--no-undefined' is now wrong for
this lib. How is that option controlled? Globally, by the configure
flag '--enable-disallowing-of-undefined-references' (ugh), but how per
lib?
build.conf contains undefined-lib-symbols = yes for libsvn_test.
Though in general, seems that disallowing undefined references is
preferable (when possible).
Or should this function be moved to libsvn_fs_util/util.c, with merely a
public wrapper placed in libsvn_fs/loader.c if required?
IIRC, libsvn_fs depends on libsvn_fs_{fs,base} depend on libsvn_fs_util.
So if code in fsfs or bdb wants to call svn_fs_validate_mergeinfo(),
then the code they call should live in libsvn_fs_util.
(This probably means Yes.)
Daniel
- Julian
On Wed, 2010-11-17, pbu...@apache.org wrote:
Log:
Reintegrate the issue-3668-3669 branch.
Re: Windows svnsync test suite failures and a clue
Paul Burba wrote on Tue, Nov 16, 2010 at 17:55:30 -0500: As some of you know I've recently had some strange failures with the svnsync tests on both trunk and 1.6.x. All the tests started failing during setup when the stdout of svnsync init was lost, e.g.: [[[ CMD: svnsync.exe initialize file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1-1 file:///C%3A/SVN/sr c-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1 --username jrandom --password rayjandom --config-dir C:\SVN\src-branc h-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config TIME = 0.314000 EXCEPTION: SVNUnexpectedStdout: [] It's expecting a Copied r0 revprops line. If you run that 'svnsync init' command manually, does it print that line? Traceback (most recent call last): File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\main.py, line 1226, in run rc = self.pred.run(**kw) File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\testcase.py, line 121, in run return self.func(sandbox) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 193, in copy_and_modify run_test(sbox, copy-and-modify.dump) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 157, in run_test run_init(dest_sbox.repo_url, repo_url) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 100, in run_init raise SVNUnexpectedStdout(output) SVNUnexpectedStdout: [] FAIL: svnsync_tests.py 1: copy and modify ]]] Then, on a lark, I tried to build 1.6.x with the --disable-shared option. The tests passed. Because this problem seemed to come and go I tired a clean rebuild with shared libs. It failed. 3 times I tried shared/static libs, each time the static lib build passed, the shared lib build failed. Does anyone have any idea what could be the problem? Obviously I'm using --disable-shared for the time being, but would like to figure out what is happening here. I don't know, but perhaps change SVN_VER_MINOR/SVN_VER_TAG to ensure the DLLs you load are the right ones? Thanks, Paul
Re: The new warning: The depth of this commit is 'empty', but copied dirs ...
On Wed, 2010-11-17, Stefan Sperling wrote: On Wed, Nov 17, 2010 at 12:04:21PM +, Julian Foad wrote: I think it would be better if we only issue this warning when the proposed commit does in fact include a copy, otherwise it's going to be too much noise. Thoughts? The warning is printed very early in the commit process, way before the commit targets are harvested. Printing the warning at a later stage is much harder to do, especially in light of backporting the change to 1.6.x because that would require API changes (the backport has happened in 1.6.13). But yes, I agree that it would be better to print the warning only if the commit harvester has actually picked up a copy. We could file an issue with 1.7.0 milestone for this, and I'll try to take a crack at it if I can find the time. Great, thanks. Issue #3752 http://subversion.tigris.org/issues/show_bug.cgi?id=3752, Warning about committing copied dir at depth 'empty'. I won't hold you to working on it, of course. - Julian
how to apply trademark symbols to Apache Subversion
Hi Legal peeps, The Subversion community updated the website to conform to the new guidelines, but we have a question on how to apply the appropriate trademark symbols. The guideline says to put a (tm) at the end of the first use of Apache Subversion, however, Subversion (alone) is a *registered* trademark. Should we just put an (r) after the first use of Subversion and leave it at that? And if that first use is Apache Subversion(r), would that still be fine? Maybe we should put an (r) after the first Apache Subversion and the first Subversion to clarify what is registered? Thanks, -g
Re: how to apply trademark symbols to Apache Subversion
On 11/17/2010 08:56 AM, Greg Stein wrote: Hi Legal peeps, The Subversion community updated the website to conform to the new guidelines, but we have a question on how to apply the appropriate trademark symbols. The guideline says to put a (tm) at the end of the first use of Apache Subversion, however, Subversion (alone) is a *registered* trademark. Should we just put an (r) after the first use of Subversion and leave it at that? And if that first use is Apache Subversion(r), would that still be fine? Maybe we should put an (r) after the first Apache Subversion and the first Subversion to clarify what is registered? I would expected that the common practice would be to, for the first use of Apache Subversion, display as Apache(tm) Subversion(r). -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: Windows svnsync test suite failures and a clue
On Wed, Nov 17, 2010 at 8:31 AM, Daniel Shahaf d...@daniel.shahaf.name wrote: Paul Burba wrote on Tue, Nov 16, 2010 at 17:55:30 -0500: As some of you know I've recently had some strange failures with the svnsync tests on both trunk and 1.6.x. All the tests started failing during setup when the stdout of svnsync init was lost, e.g.: [[[ CMD: svnsync.exe initialize file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1-1 file:///C%3A/SVN/sr c-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1 --username jrandom --password rayjandom --config-dir C:\SVN\src-branc h-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config TIME = 0.314000 EXCEPTION: SVNUnexpectedStdout: [] It's expecting a Copied r0 revprops line. If you run that 'svnsync init' command manually, does it print that line? Hi Daniel, Yes, if I break the test and run the init manually I see the expected output, that is part of what makes this so perplexing: C:\SVN\src-branch-1.6.xrun.python.test.DEBUG.bat svnsync 1 C:\SVN\src-branch-1.6.xecho off EXCEPTION: Failure: Break on init!!! Traceback (most recent call last): File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\main.py, line 1226, in run rc = self.pred.run(**kw) File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\testcase.py, line 121, in run return self.func(sandbox) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 194, in copy_and_modify run_test(sbox, copy-and-modify.dump) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 158, in run_test run_init(dest_sbox.repo_url, repo_url) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 93, in run_init raise svntest.Failure(Break on init!!!) Failure: Break on init!!! FAIL: svnsync_tests.py 1: copy and modify C:\SVN\src-branch-1.6.xsvnsync initialize file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1-1 file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1 --username jrandom --password rayjandom Copied properties for revision 0. Ah, but here is something interesting, if I manually run the initialization and redirect the output to a file, the output is lost! C:\SVN\src-branch-1.6.xrun.python.test.DEBUG.bat svnsync 1 C:\SVN\src-branch-1.6.xecho off EXCEPTION: Failure: Break on init!!! Traceback (most recent call last): File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\main.py, line 1226, in run rc = self.pred.run(**kw) File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\testcase.py, line 121, in run return self.func(sandbox) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 194, in copy_and_modify run_test(sbox, copy-and-modify.dump) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 158, in run_test run_init(dest_sbox.repo_url, repo_url) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 93, in run_init raise svntest.Failure(Break on init!!!) Failure: Break on init!!! FAIL: svnsync_tests.py 1: copy and modify C:\SVN\src-branch-1.6.xsvnsync initialize file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1-1 file :///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1 --username jrandom --password rayjandom svnsync.out.txt C:\SVN\src-branch-1.6.xtype svnsync.out.txt C:\SVN\src-branch-1.6.x ^^^ Where's the output? The init happened: C:\SVN\src-branch-1.6.xsvnsync initialize file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1-1 file :///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1 --username jrandom --password rayjandom ..\..\..\subversion\svnsync\main.c:736: (apr_err=22) svnsync: Destination repository is already synchronizing from 'file:///C%3A/SVN/src-branch-1.6.x/Debug/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-1' Traceback (most recent call last): File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\main.py, line 1226, in run rc = self.pred.run(**kw) File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\testcase.py, line 121, in run return self.func(sandbox) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 193, in copy_and_modify run_test(sbox, copy-and-modify.dump) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 157, in run_test run_init(dest_sbox.repo_url, repo_url) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 100, in
Re: svn commit: r1035894 - in /subversion/trunk: ./ subversion/include/ subversion/include/private/ subversion/libsvn_client/ subversion/libsvn_fs/ subversion/libsvn_fs_base/ subversion/libsvn_fs_fs/
On Wed, Nov 17, 2010 at 5:45 AM, Julian Foad julian.f...@wandisco.com wrote: Hi Paul. On Wed, 2010-11-17, pbu...@apache.org wrote: Log: Reintegrate the issue-3668-3669 branch. If you could insert a summary of this change here in the log message, that would be helpful. Sure thing, I added a bit more explanation to the log message. FWIW I always figured the log messages of the feature branch served this purpose, but your implied point is certainly valid, it is a lot more convenient for everybody other than the branch author(s)! For your convenience, here is the new log message: [[[ Reintegrate the issue-3668-3669 branch. This fixes issue #3668 'inheritance can result in self-referential mergeinfo' and issue #3669 'inheritance can result in mergeinfo describing nonexistent sources'. The work for both of these issues was done in tandem for two reasons: First, because they are very similar, i.e. both are variations of the problem where merge tracking's simple inheritance model resulted in mergeinfo that contained invalid path-revs (either self-referential or simply non-existent). Second, the fix for issue #3668, while simple, behaves badly in cases where it is corrected but issue #3669 in not, see http://subversion.tigris.org/issues/show_bug.cgi?id=3668#desc5 The core of the issue #3669 fix is two-fold: First, rev svn_ra_get_mergeinfo() so it can request that the server validate any inherited mergeinfo it returns and second, rev svn_repos_fs_get_mergeinfo() so the server can actually perform this validation. Note that since an RA API was revved, a lot of the noise in this change is simply updating the plumbing of the various RA providers. Any potential reviewers wanting to see the heart of this change should probably start here: libsvn_fs/fs-loader.c:svn_fs_validate_mergeinfo(): Does the heavy lifting of validating mergeinfo. libsvn_client/merge.c:get_invalid_inherited_mergeinfo(): Indirectly uses the new RA API to determine what portion of WC path's inherited mergeinfo is invalid. ]]] let me know if that's not what you had in mind. I see it's ... rather large. As I mentioned in the new log, a lot of the code churn is RA plumbing changes...hmmm, I guess this is exhibit 'A' as to why a more thorough log message is helpful! Thanks, Paul Modified: subversion/trunk/ (props changed) subversion/trunk/subversion/include/private/svn_dav_protocol.h subversion/trunk/subversion/include/svn_fs.h subversion/trunk/subversion/include/svn_ra.h subversion/trunk/subversion/include/svn_repos.h subversion/trunk/subversion/libsvn_client/copy.c subversion/trunk/subversion/libsvn_client/merge.c subversion/trunk/subversion/libsvn_client/mergeinfo.c subversion/trunk/subversion/libsvn_client/mergeinfo.h subversion/trunk/subversion/libsvn_fs/fs-loader.c subversion/trunk/subversion/libsvn_fs/fs-loader.h subversion/trunk/subversion/libsvn_fs_base/tree.c subversion/trunk/subversion/libsvn_fs_fs/tree.c subversion/trunk/subversion/libsvn_ra/deprecated.c subversion/trunk/subversion/libsvn_ra/ra_loader.c subversion/trunk/subversion/libsvn_ra/ra_loader.h subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c subversion/trunk/subversion/libsvn_ra_neon/mergeinfo.c subversion/trunk/subversion/libsvn_ra_neon/options.c subversion/trunk/subversion/libsvn_ra_neon/ra_neon.h subversion/trunk/subversion/libsvn_ra_serf/mergeinfo.c subversion/trunk/subversion/libsvn_ra_serf/options.c subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h subversion/trunk/subversion/libsvn_ra_svn/client.c subversion/trunk/subversion/libsvn_repos/fs-wrap.c subversion/trunk/subversion/mod_dav_svn/reports/mergeinfo.c subversion/trunk/subversion/svnserve/serve.c subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py subversion/trunk/subversion/tests/cmdline/merge_tests.py
Re: how to apply trademark symbols to Apache Subversion
On Wed, Nov 17, 2010 at 09:08, C. Michael Pilato cmpil...@collab.net wrote: On 11/17/2010 08:56 AM, Greg Stein wrote: Hi Legal peeps, The Subversion community updated the website to conform to the new guidelines, but we have a question on how to apply the appropriate trademark symbols. The guideline says to put a (tm) at the end of the first use of Apache Subversion, however, Subversion (alone) is a *registered* trademark. Should we just put an (r) after the first use of Subversion and leave it at that? And if that first use is Apache Subversion(r), would that still be fine? Maybe we should put an (r) after the first Apache Subversion and the first Subversion to clarify what is registered? I would expected that the common practice would be to, for the first use of Apache Subversion, display as Apache(tm) Subversion(r). Well, we have three terms/phrases in play here: * Apache(tm) * Subversion(r) * Apache Subversion ... ?? The first two are easy. But we also want to reserve Apache Subversion, much like we have Apache Tomcat(tm) (where the (tm) is implied for the two-word phrase) Cheers, -g
Re: how to apply trademark symbols to Apache Subversion
On 11/17/2010 10:35 AM, Greg Stein wrote: On Wed, Nov 17, 2010 at 09:08, C. Michael Pilato cmpil...@collab.net wrote: On 11/17/2010 08:56 AM, Greg Stein wrote: Hi Legal peeps, The Subversion community updated the website to conform to the new guidelines, but we have a question on how to apply the appropriate trademark symbols. The guideline says to put a (tm) at the end of the first use of Apache Subversion, however, Subversion (alone) is a *registered* trademark. Should we just put an (r) after the first use of Subversion and leave it at that? And if that first use is Apache Subversion(r), would that still be fine? Maybe we should put an (r) after the first Apache Subversion and the first Subversion to clarify what is registered? I would expected that the common practice would be to, for the first use of Apache Subversion, display as Apache(tm) Subversion(r). Well, we have three terms/phrases in play here: * Apache(tm) * Subversion(r) * Apache Subversion ... ?? The first two are easy. But we also want to reserve Apache Subversion, much like we have Apache Tomcat(tm) (where the (tm) is implied for the two-word phrase) I see your point, but you can't use Apache Subversion without running into the marks for each (both!) of those words. Is there any value in trying to distinguish between tm(Apache Subversion) and: tm(Apache) + r(Subversion) ? -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: svn commit: r1036138 - /subversion/trunk/subversion/svnrdump/load_editor.c
- err = svn_error_create( - SVN_ERR_UNSUPPORTED_FEATURE, NULL, - _(Target server does not support atomic revision property -edits; consider upgrading it to 1.7 or using an external -locking program)); + /* Pre-1.7 servers can't lock without a race condition. (Issue #3546) */ + svn_error_t *err = +svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL, + _(Target server does not support atomic revision + property edits; consider upgrading it to 1.7 + or using an external locking program)); Hmm. I realize this is preexisting code, but the bit about external locking doesn't apply to svnrdump, since unlike svnsync it doesn't have a --use-external-locking flag. Daniel
Re: Issue triage: issues 3429 and 3474
Johan Corveleyn wrote on Wed, Nov 17, 2010 at 01:25:24 +0100: On Mon, Nov 8, 2010 at 1:02 PM, Philip Martin philip.mar...@wandisco.com wrote: Johan Corveleyn jcor...@gmail.com writes: Or, maybe the best approach: I could add a regression test for these issues, so we can all be sure that they are fixed (and remain fixed), after which they can be marked as fixed. Yes, please. Are there any existing XFAIL tests that apply? They sometimes don't XPASS automatically when the bug is fixed because the test expectation is wrong. Doh, it seems that issue #3474 already has a test, which PASSes (added by Bert, which he mentioned in a comment in the issue): PASS: copy_tests.py 81: copy of new dir with copied file keeps history This is exactly what issue #3474 is about. Bert added the test as XFAIL in r938071, and it was marked PASS by you, Philip, in r955334. So, I guess this wraps up that issue: can someone mark it as resolved? Could you mark it as resolved? Or don't you have the necessary permissions on tigris? There was a slight confusion when I read the test code, because a comment still talks about the tests as Currently this fails because The following patch removes that obsolete comment: [[[ Index: subversion/tests/cmdline/copy_tests.py === --- subversion/tests/cmdline/copy_tests.py(revision 1035851) +++ subversion/tests/cmdline/copy_tests.py(working copy) @@ -4358,8 +4358,6 @@ def copy_added_dir_with_copy(sbox): 'NewDir2/mu': Item(status='A ', copied='+', wc_rev='-'), }) - # Currently this fails because NewDir2/mu loses its history in the copy - # from NewDir to NewDir2 svntest.actions.run_and_verify_status(wc_dir, expected_status) +1, but could you add a link to the issue (in a comment) while you're there? ]]] For issue #3429, I'll try to write a regression test myself, and post a patch for it in a new thread. Cheers, -- Johan
Re: svn commit: r1036138 - /subversion/trunk/subversion/svnrdump/load_editor.c
On 11/17/2010 01:07 PM, Daniel Shahaf wrote: - err = svn_error_create( - SVN_ERR_UNSUPPORTED_FEATURE, NULL, - _(Target server does not support atomic revision property -edits; consider upgrading it to 1.7 or using an external -locking program)); + /* Pre-1.7 servers can't lock without a race condition. (Issue #3546) */ + svn_error_t *err = +svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL, + _(Target server does not support atomic revision + property edits; consider upgrading it to 1.7 + or using an external locking program)); Hmm. I realize this is preexisting code, but the bit about external locking doesn't apply to svnrdump, since unlike svnsync it doesn't have a --use-external-locking flag. I *knew* there was something I had made a mental note to remove. When I went back later to do so, I'd forgotten what it was. Thanks for the reminder! Will fix in a bit -- got another change in progress here. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Backport review - r1035992 in branches/1.6.x-r917523: ./ subversion/mod_dav_svn/mirror.c
I've reviewed and tested this change. Here are the results. Executive
summary: basically works in the simple case, but I have too many
reservations to give a +1.
The patch attempts to fix DAV mirroring when the Location directive
value contains a space, such as Location /repo 1. For the original
log message of r917523 on trunk, see below [1].
I tested this change using the attached 'dav-mirror-autocheck.sh' [2].
Using dav-mirror-autocheck.sh configured internally as:
MASTER_LOCATION=repo 1
SLAVE_LOCATION=repo 1
SYNC_LOCATION=sync
[...]
# slave 'normal' location
Location /${SLAVE_LOCATION}
# slave 'sync' location
Location /${SYNC_LOCATION}
# master
Location /${MASTER_LOCATION}
the script's test passed. Without this patch, the svnsync commits
occurred only on the slave repository and so the script's test failed
with:
dav-mirror-autocheck.sh: FAIL: master, slave are at rev 0, 4, not 4
So in terms of achieving its goal of allowing a space in a Location
directive, it works.
I then tried
SYNC_LOCATION=sync 1
and that didn't work - the initial sync failed with:
svnsync: Server sent unexpected return value (405 Method Not Allowed) in
response to PROPPATCH request for '/sync%25201/!svn/bln/0'
That makes me wonder whether the patch is missing a bit that it should
include, or whether in fact that similar-looking case is handled
entirely separately.
Reviewing the code:
On Wed, 2010-11-17, julianf...@apache.org wrote:
New Revision: 1035992
URL: http://svn.apache.org/viewvc?rev=1035992view=rev
Log:
On the 1.6.x-r917523 branch: Merge r917523 from trunk, resolving conflicts.
Modified:
subversion/branches/1.6.x-r917523/ (props changed)
subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c
Modified: subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c
URL:
http://svn.apache.org/viewvc/subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c?rev=1035992r1=1035991r2=1035992view=diff
==
--- subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c
(original)
+++ subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c Wed Nov
17 11:57:58 2010
@@ -40,8 +40,10 @@ static void proxy_request_fixup(request_
r-proxyreq = PROXYREQ_REVERSE;
r-uri = r-unparsed_uri;
-r-filename = apr_pstrcat(r-pool, proxy:, master_uri,
- uri_segment, NULL);
+r-filename = (char *) svn_path_uri_encode(apr_pstrcat(r-pool, proxy:,
+ master_uri,
+ uri_segment,
+ NULL), r-pool);
This change effectively says, We are deciding that the MASTER_URI and
URI_SEGMENT arguments to this local function should be passed as
unencoded strings.
r-handler = proxy-server;
ap_add_output_filter(LocationRewrite, NULL, r, r-connection);
ap_add_output_filter(ReposRewrite, NULL, r, r-connection);
@@ -74,7 +76,7 @@ int dav_svn__proxy_merge_fixup(request_r
of the repository isn't guaranteed to have on hand. */
if (r-method_number == M_PROPFIND ||
r-method_number == M_GET) {
-seg = ap_strstr(r-unparsed_uri, root_dir);
+seg = ap_strstr(r-uri, root_dir);
if (seg ap_strstr_c(seg,
apr_pstrcat(r-pool, special_uri,
/wrk/, NULL))) {
@@ -87,7 +89,7 @@ int dav_svn__proxy_merge_fixup(request_r
/* If this is a write request aimed at a public URI (such as
MERGE, LOCK, UNLOCK, etc.) or any as-yet-unhandled request
using a special URI, we have to doctor it a bit for proxying. */
-seg = ap_strstr(r-unparsed_uri, root_dir);
+seg = ap_strstr(r-uri, root_dir);
if (seg (r-method_number == M_MERGE ||
r-method_number == M_LOCK ||
r-method_number == M_UNLOCK ||
@@ -134,9 +136,11 @@ apr_status_t dav_svn__location_in_filter
ctx = f-ctx = apr_pcalloc(r-pool, sizeof(*ctx));
apr_uri_parse(r-pool, master_uri, ctx-uri);
-ctx-remotepath = ctx-uri.path;
+/* We are url encoding the current url and the master url
+ as incoming(from client) request body has it encoded already. */
+ctx-remotepath = svn_path_uri_encode(ctx-uri.path, r-pool);
Note: ctx-uri is not used elsewhere, so is acting here as a temporary
variable. So this change is equivalent to having encoded MASTER_URI,
which is the return value of dav_svn__get_master_uri().
ctx-remotepath_len = strlen(ctx-remotepath);
-ctx-localpath = dav_svn__get_root_dir(r);
+ctx-localpath = svn_path_uri_encode(dav_svn__get_root_dir(r),
r-pool);
Here we
Re: svn commit: r1036138 - /subversion/trunk/subversion/svnrdump/load_editor.c
On 11/17/2010 01:22 PM, C. Michael Pilato wrote: On 11/17/2010 01:07 PM, Daniel Shahaf wrote: Hmm. I realize this is preexisting code, but the bit about external locking doesn't apply to svnrdump, since unlike svnsync it doesn't have a --use-external-locking flag. I *knew* there was something I had made a mental note to remove. When I went back later to do so, I'd forgotten what it was. Thanks for the reminder! Will fix in a bit -- got another change in progress here. r1036168. Thanks again. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: Backport review - r1035992 in branches/1.6.x-r917523: ./ subversion/mod_dav_svn/mirror.c
On Wed, 2010-11-17 at 18:57 +, Julian Foad wrote:
I've reviewed and tested this change. Here are the results. Executive
summary: basically works in the simple case, but I have too many
reservations to give a +1.
A couple more points:
* I mentioned we should consider canonicalization and autoescaping
instead of this unconditional encoding. In fact I think it's important
to do it that way for back-compat; the patch as it is will break anybody
using percent-encoding.
* I intend to fix this issue and my concerns, having got this far. But
first I'll need to check how this interacts with the canonicalize
patch that was applied to trunk in r916286 before this one and is also
proposed for back-port.
- Julian
The patch attempts to fix DAV mirroring when the Location directive
value contains a space, such as Location /repo 1. For the original
log message of r917523 on trunk, see below [1].
I tested this change using the attached 'dav-mirror-autocheck.sh' [2].
Using dav-mirror-autocheck.sh configured internally as:
MASTER_LOCATION=repo 1
SLAVE_LOCATION=repo 1
SYNC_LOCATION=sync
[...]
# slave 'normal' location
Location /${SLAVE_LOCATION}
# slave 'sync' location
Location /${SYNC_LOCATION}
# master
Location /${MASTER_LOCATION}
the script's test passed. Without this patch, the svnsync commits
occurred only on the slave repository and so the script's test failed
with:
dav-mirror-autocheck.sh: FAIL: master, slave are at rev 0, 4, not 4
So in terms of achieving its goal of allowing a space in a Location
directive, it works.
I then tried
SYNC_LOCATION=sync 1
and that didn't work - the initial sync failed with:
svnsync: Server sent unexpected return value (405 Method Not Allowed) in
response to PROPPATCH request for '/sync%25201/!svn/bln/0'
That makes me wonder whether the patch is missing a bit that it should
include, or whether in fact that similar-looking case is handled
entirely separately.
Reviewing the code:
On Wed, 2010-11-17, julianf...@apache.org wrote:
New Revision: 1035992
URL: http://svn.apache.org/viewvc?rev=1035992view=rev
Log:
On the 1.6.x-r917523 branch: Merge r917523 from trunk, resolving conflicts.
Modified:
subversion/branches/1.6.x-r917523/ (props changed)
subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c
Modified: subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c
URL:
http://svn.apache.org/viewvc/subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c?rev=1035992r1=1035991r2=1035992view=diff
==
--- subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c
(original)
+++ subversion/branches/1.6.x-r917523/subversion/mod_dav_svn/mirror.c Wed
Nov 17 11:57:58 2010
@@ -40,8 +40,10 @@ static void proxy_request_fixup(request_
r-proxyreq = PROXYREQ_REVERSE;
r-uri = r-unparsed_uri;
-r-filename = apr_pstrcat(r-pool, proxy:, master_uri,
- uri_segment, NULL);
+r-filename = (char *) svn_path_uri_encode(apr_pstrcat(r-pool,
proxy:,
+ master_uri,
+ uri_segment,
+ NULL), r-pool);
This change effectively says, We are deciding that the MASTER_URI and
URI_SEGMENT arguments to this local function should be passed as
unencoded strings.
r-handler = proxy-server;
ap_add_output_filter(LocationRewrite, NULL, r, r-connection);
ap_add_output_filter(ReposRewrite, NULL, r, r-connection);
@@ -74,7 +76,7 @@ int dav_svn__proxy_merge_fixup(request_r
of the repository isn't guaranteed to have on hand. */
if (r-method_number == M_PROPFIND ||
r-method_number == M_GET) {
-seg = ap_strstr(r-unparsed_uri, root_dir);
+seg = ap_strstr(r-uri, root_dir);
if (seg ap_strstr_c(seg,
apr_pstrcat(r-pool, special_uri,
/wrk/, NULL))) {
@@ -87,7 +89,7 @@ int dav_svn__proxy_merge_fixup(request_r
/* If this is a write request aimed at a public URI (such as
MERGE, LOCK, UNLOCK, etc.) or any as-yet-unhandled request
using a special URI, we have to doctor it a bit for proxying.
*/
-seg = ap_strstr(r-unparsed_uri, root_dir);
+seg = ap_strstr(r-uri, root_dir);
if (seg (r-method_number == M_MERGE ||
r-method_number == M_LOCK ||
r-method_number == M_UNLOCK ||
@@ -134,9 +136,11 @@ apr_status_t dav_svn__location_in_filter
ctx = f-ctx = apr_pcalloc(r-pool, sizeof(*ctx));
apr_uri_parse(r-pool, master_uri,
Re: svn commit: r1036078 - in /subversion/trunk/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h libsvn_fs_base/fs.c libsvn_fs_base/tree.c libsvn_fs_base/tree.h libsvn_fs_fs/fs
On 11/17/2010 07:26 AM, cmpil...@apache.org wrote: Author: cmpilato Date: Wed Nov 17 15:26:33 2010 New Revision: 1036078 URL: http://svn.apache.org/viewvc?rev=1036078view=rev Log: Fix a library dependency issue which was causing build failures. (As a general rule, code inside the FS providers shouldn't call back into the public FS API.) Sadly, the constraints of our system (namely the BDB side of things) necessitate code duplication rather than sharing. Could we do something like having the .c files #include other .c files that share common code? Blair
Re: [Issue 3716] svnrdump does not support cancellation
Thanks! cmpil...@tigris.org wrote on Wed, Nov 17, 2010 at 11:10:57 -0800: http://subversion.tigris.org/issues/show_bug.cgi?id=3716 User cmpilato changed the following: What|Old value |New value Status|REOPENED |RESOLVED Resolution| |FIXED
Re: svn commit: r1036078 - in /subversion/trunk/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h libsvn_fs_base/fs.c libsvn_fs_base/tree.c libsvn_fs_base/tree.h libsvn_fs_fs/fs
On 11/17/2010 02:26 PM, Blair Zajac wrote: On 11/17/2010 07:26 AM, cmpil...@apache.org wrote: Author: cmpilato Date: Wed Nov 17 15:26:33 2010 New Revision: 1036078 URL: http://svn.apache.org/viewvc?rev=1036078view=rev Log: Fix a library dependency issue which was causing build failures. (As a general rule, code inside the FS providers shouldn't call back into the public FS API.) Sadly, the constraints of our system (namely the BDB side of things) necessitate code duplication rather than sharing. Could we do something like having the .c files #include other .c files that share common code? If we do so, we have use only functions common to both backends. Such functions only exist in the libsvn_fs library itself. But using libsvn_fs_ functions means the BDB code is re-entering the FS API, which is dangerous in many cases (due to the problem of nested Berkeley DB transactions, which cause hangs). The only really enjoyable solution is for the business logic of our backends to be combined and made to use a generic storage logic API which differs from backend to backend. Something to strongly consider / require for FS-NG. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
1.6.14 tarballs up for testing/signing
1.6.14 tarballs are up for testing and signing. The magic revision is r1036150: http://people.apache.org/~hwright/svn/1.6.14/ In addition to sending your signatures to the mailing list, please use the new signature collection script, located here: http://work.hyrumwright.org/pub/svn/collect_sigs.py The script will automatically check the signature for validity, and then add it to the list of signatures. If you haven't signed a release in a while, your key may not be on the keyring; if so, please let me know and I shall add it. If you get an error, please let me know. Please do not abuse the script, as it is pretty fragile right now (the source, however, it located in the repo at tools/dist/, and any updates made there will propagate to the live site fairly quickly). Hopefully the collection script works well, and barring any terrible failures, I plan on taking the signatures collected by it for the release. But I'd still like sigs sent to the list this time, you know, just in case. Thanks for the patience! 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 November 19. -Hyrum
test failure 1.6.14 (svnsync test 29 with bdb; copyfrom)
With BDB, svnsync crashes as follows during during svnsync test 29. It only happens with BDB. Running ./svnsync_tests.py 29 --fs-type=bdb triggers the problem for me on the 1.6.x branch, too. I guess this could be related to this backport item: * improve svnsync handling of dir copies (r962377, -8) Details follow: SVNProcessTerminatedBySignal FAIL: svnsync_tests.py 29: descending into replaced dir looks in src #0 0x00020f08ed0a in kill () from /usr/lib/libc.so.58.0 (gdb) up #1 0x00020f0e5bb1 in abort () at /usr/src/lib/libc/stdlib/abort.c:68 68 (void)kill(getpid(), SIGABRT); (gdb) #2 0x0002020a87fd in svn_error_abort_on_malfunction (can_return=Could not find the frame base for svn_error_abort_on_malfunction. ) at subversion/libsvn_subr/error.c:525 525 abort(); (gdb) #3 0x0002020a884e in svn_error__malfunction (can_return=1, file=0x201d7ff68 subversion/libsvn_repos/replay.c, line=224, expr=0x201d7ff50 change-copyfrom_known) at subversion/libsvn_subr/error.c:548 548 return malfunction_handler(can_return, file, line, expr); (gdb) #4 0x000201c7490a in add_subdir (source_root=0x20dbb60a0, target_root=0x2013790a0, editor=0x20aa28868, edit_baton=0x20aa288e8, path=0x2090df2a1 trunk/H, parent_baton=0x20a8a90a0, source_path=0x2075150d0 /trunk/A, authz_read_func=0, authz_read_baton=0x0, changed_paths=0x20aa28968, pool=0x207515028, dir_baton=0x7f7ec3b8) at subversion/libsvn_repos/replay.c:224 224 SVN_ERR_ASSERT(change-copyfrom_known); (gdb) p change-copyfrom_known $1 = 0 (gdb) bt #0 0x00020f08ed0a in kill () from /usr/lib/libc.so.58.0 #1 0x00020f0e5bb1 in abort () at /usr/src/lib/libc/stdlib/abort.c:68 #2 0x0002020a87fd in svn_error_abort_on_malfunction (can_return=Could not find the frame base for svn_error_abort_on_malfunction. ) at subversion/libsvn_subr/error.c:525 #3 0x0002020a884e in svn_error__malfunction (can_return=1, file=0x201d7ff68 subversion/libsvn_repos/replay.c, line=224, expr=0x201d7ff50 change-copyfrom_known) at subversion/libsvn_subr/error.c:548 #4 0x000201c7490a in add_subdir (source_root=0x20dbb60a0, target_root=0x2013790a0, editor=0x20aa28868, edit_baton=0x20aa288e8, path=0x2090df2a1 trunk/H, parent_baton=0x20a8a90a0, source_path=0x2075150d0 /trunk/A, authz_read_func=0, authz_read_baton=0x0, changed_paths=0x20aa28968, pool=0x207515028, dir_baton=0x7f7ec3b8) at subversion/libsvn_repos/replay.c:224 #5 0x000201c753b0 in path_driver_cb_func (dir_baton=0x7f7ec3b8, parent_baton=0x20a8a90a0, callback_baton=0x7f7ec440, path=0x2090df2a1 trunk/H, pool=0x207515028) at subversion/libsvn_repos/replay.c:465 #6 0x00020fb59ec9 in svn_delta_path_driver (editor=0x20aa28868, edit_baton=0x20aa288e8, revision=-1, paths=0x20aa28938, callback_func=0x201c74ea5 path_driver_cb_func, callback_baton=0x7f7ec440, pool=0x207512028) at subversion/libsvn_delta/path_driver.c:245 #7 0x000201c76123 in svn_repos_replay2 (root=0x2013790a0, base_path=0x2090de789 trunk/H, low_water_mark=0, send_deltas=1, editor=0x20aa28868, edit_baton=0x20aa288e8, authz_read_func=0, authz_read_baton=0x0, pool=0x207512028) at subversion/libsvn_repos/replay.c:791 #8 0x0002052e5c00 in svn_ra_local__replay (session=0x2028e1a78, revision=3, low_water_mark=0, send_deltas=1, editor=0x20aa28868, edit_baton=0x20aa288e8, pool=0x207512028) at subversion/libsvn_ra_local/ra_plugin.c:1346 #9 0x00020bb35e07 in svn_ra_replay (session=0x2028e1a78, revision=3, low_water_mark=0, text_deltas=1, editor=0x20aa28868, edit_baton=0x20aa288e8, pool=0x207512028) at subversion/libsvn_ra/ra_loader.c:1013 #10 0x00020bb35f96 in svn_ra_replay_range (session=0x2028e1a78, start_revision=1, end_revision=3, low_water_mark=0, text_deltas=1, revstart_func=0x404d31 replay_rev_started, revfinish_func=0x405003 replay_rev_finished, replay_baton=0x2090de850, pool=0x20497d028) at subversion/libsvn_ra/ra_loader.c:1056 #11 0x00405784 in do_synchronize (to_session=0x209163808, baton=0x209163600, pool=0x20497d028) at subversion/svnsync/main.c:1781 #12 0x00402d07 in with_locked (session=0x209163808, func=0x40531a do_synchronize, baton=0x209163600, pool=0x20497d028) at subversion/svnsync/main.c:327 #13 0x004059d8 in synchronize_cmd (os=0x20497d188, b=0x7f7ec900, pool=0x20497d028) at subversion/svnsync/main.c:1824 #14 0x00406ed3 in main (argc=9, argv=0x7f7eca48) at subversion/svnsync/main.c:2381 (gdb)
Re: test failure 1.6.14 (svnsync test 29 with bdb; copyfrom)
Fails for me too: C:\SVN\src-branch-1.6.xrun.python.test.RELEASE.bat svnsync 29 -v --fs-type bdb C:\SVN\src-branch-1.6.xset TESTNAME=svnsync C:\SVN\src-branch-1.6.xset CONFIG=Release C:\SVN\src-branch-1.6.xif not exist Release\subversion\tests\cmdline mkdir Release\subversion\tests\cmdline C:\SVN\src-branch-1.6.xpushd Release\subversion\tests\cmdline C:\SVN\src-branch-1.6.x\Release\subversion\tests\cmdlinepython C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py 29 -v --fs-type bdb CMD: svnadmin.exe create svn-test-work\local_tmp\repos --bdb-txn-nosync --fs-type=bdb TIME = 0.235000 CMD: svn.exe import -m Log message for revision 1. svn-test-work\local_tmp\greekfiles file:///C%3A/SVN/src-branch-1.6.x/Release/subversion/tests/cmdline/sv n-test-work/local_tmp/repos --config-dir C:\SVN\src-branch-1.6.x\Release\subversion\tests\cmdline\svn-test-work\local_tmp\config --password rayjandom --no-au th-cache --username jrandom TIME = 0.073000 Adding svn-test-work\local_tmp\greekfiles\A Adding svn-test-work\local_tmp\greekfiles\A\B Adding svn-test-work\local_tmp\greekfiles\A\B\lambda Adding svn-test-work\local_tmp\greekfiles\A\B\E Adding svn-test-work\local_tmp\greekfiles\A\B\E\alpha Adding svn-test-work\local_tmp\greekfiles\A\B\E\beta Adding svn-test-work\local_tmp\greekfiles\A\B\F Adding svn-test-work\local_tmp\greekfiles\A\mu Adding svn-test-work\local_tmp\greekfiles\A\C Adding svn-test-work\local_tmp\greekfiles\A\D Adding svn-test-work\local_tmp\greekfiles\A\D\gamma Adding svn-test-work\local_tmp\greekfiles\A\D\G Adding svn-test-work\local_tmp\greekfiles\A\D\G\pi Adding svn-test-work\local_tmp\greekfiles\A\D\G\rho Adding svn-test-work\local_tmp\greekfiles\A\D\G\tau Adding svn-test-work\local_tmp\greekfiles\A\D\H Adding svn-test-work\local_tmp\greekfiles\A\D\H\chi Adding svn-test-work\local_tmp\greekfiles\A\D\H\omega Adding svn-test-work\local_tmp\greekfiles\A\D\H\psi Adding svn-test-work\local_tmp\greekfiles\iota Committed revision 1. CMD: svnadmin.exe create svn-test-work\repositories\svnsync_tests-29 --bdb-txn-nosync --fs-type=bdb TIME = 0.141000 CMD: svnadmin.exe load --force-uuid --quiet svn-test-work\repositories\svnsync_tests-29 TIME = 0.063000 CMD: svnadmin.exe create svn-test-work\repositories\svnsync_tests-29-1 --bdb-txn-nosync --fs-type=bdb TIME = 0.161000 CMD: svnlook.exe uuid svn-test-work\repositories\svnsync_tests-29 TIME = 0.025000 b105e986-03ce-ba4b-b01a-b7a10dd0b7f5 CMD: svnadmin.exe setuuid svn-test-work\repositories\svnsync_tests-29-1 b105e986-03ce-ba4b-b01a-b7a10dd0b7f5 TIME = 0.026000 CMD: svnsync.exe initialize file:///C%3A/SVN/src-branch-1.6.x/Release/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-29-1 file:///C%3A/SVN /src-branch-1.6.x/Release/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-29/trunk/H --username jrandom --password rayjandom --config-dir C: \SVN\src-branch-1.6.x\Release\subversion\tests\cmdline\svn-test-work\local_tmp\config TIME = 0.314000 Copied properties for revision 0. CMD: svnsync.exe synchronize file:///C%3A/SVN/src-branch-1.6.x/Release/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-29-1 --username jrand om --password rayjandom --config-dir C:\SVN\src-branch-1.6.x\Release\subversion\tests\cmdline\svn-test-work\local_tmp\config CMD: C:\SVN\src-branch-1.6.x\Rele ase\subversion\svnsync\svnsync.exe synchronize file:///C%3A/SVN/src-branch-1.6.x/Release/subversion/tests/cmdline/svn-test-work/repositories/svnsync_tests-29-1 --username jrandom --password rayjandom --config-dir C:\SVN\src-branch-1.6.x\Release\subversion\tests\cmdline\svn-test-work\local_tmp\config exited with 3 TIME = 7.829000 Committed revision 1. Copied properties for revision 1. Committed revision 2. Copied properties for revision 2. Transmitting file data .svn: In file '..\..\..\subversion\libsvn_repos\replay.c' line 224: assertion failed (change-copyfrom_known) This application has requested the Runtime to terminate it in an unusual way. Please contact the application's support team for more information. EXCEPTION: SVNUnexpectedStderr: [svn: In file '..\\..\\..\\subversion\\libsvn_repos\\replay.c' line 224: assertion failed (change-copyfrom_known)\n, '\n', 'T his application has requested the Runtime to terminate it in an unusual way.\n', Please contact the application's support team for more information.\n] Traceback (most recent call last): File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\main.py, line 1227, in run rc = self.pred.run(**kw) File C:\SVN\src-branch-1.6.x\subversion\tests\cmdline\svntest\testcase.py, line 121, in run return self.func(sandbox) File C:\SVN\src-branch-1.6.x\\subversion\tests\cmdline\svnsync_tests.py, line 768, in descend_into_replace exp_dump_file_name = descend_into_replace.expected.dump) File
Re: test failure 1.6.14 (svnsync test 29 with bdb; copyfrom)
Stefan Sperling wrote on Thu, Nov 18, 2010 at 00:54:42 +0100:
With BDB, svnsync crashes as follows during during svnsync test 29.
It only happens with BDB. Running ./svnsync_tests.py 29 --fs-type=bdb
triggers the problem for me on the 1.6.x branch, too.
I guess this could be related to this backport item:
* improve svnsync handling of dir copies (r962377, -8)
You're being polite --- this backport item added the assert. :-)
Details follow:
SVNProcessTerminatedBySignal
FAIL: svnsync_tests.py 29: descending into replaced dir looks in src
...
#4 0x000201c7490a in add_subdir (source_root=0x20dbb60a0,
target_root=0x2013790a0, editor=0x20aa28868, edit_baton=0x20aa288e8,
path=0x2090df2a1 trunk/H, parent_baton=0x20a8a90a0,
source_path=0x2075150d0 /trunk/A, authz_read_func=0,
authz_read_baton=0x0, changed_paths=0x20aa28968, pool=0x207515028,
dir_baton=0x7f7ec3b8) at subversion/libsvn_repos/replay.c:224
224 SVN_ERR_ASSERT(change-copyfrom_known);
FWIW, the code reads:
226 else if (change-change_kind == svn_fs_path_change_replace)
227 {
228 /* ### Can this assert fail? */
229 SVN_ERR_ASSERT(change-copyfrom_known);
230 copyfrom_path = change-copyfrom_path;
231 copyfrom_rev = change-copyfrom_rev;
232 }
So, yeah, I should have believed that API doc string which said the
copyfrom might not be known :P.
I wrote this, so I'll be happy to help investigating/fix the failures.
Daniel
Re: Issue triage: issues 3429 and 3474
On Wed, Nov 17, 2010 at 7:14 PM, Daniel Shahaf d...@daniel.shahaf.name wrote: Johan Corveleyn wrote on Wed, Nov 17, 2010 at 01:25:24 +0100: On Mon, Nov 8, 2010 at 1:02 PM, Philip Martin philip.mar...@wandisco.com wrote: Johan Corveleyn jcor...@gmail.com writes: Or, maybe the best approach: I could add a regression test for these issues, so we can all be sure that they are fixed (and remain fixed), after which they can be marked as fixed. Yes, please. Are there any existing XFAIL tests that apply? They sometimes don't XPASS automatically when the bug is fixed because the test expectation is wrong. Doh, it seems that issue #3474 already has a test, which PASSes (added by Bert, which he mentioned in a comment in the issue): PASS: copy_tests.py 81: copy of new dir with copied file keeps history This is exactly what issue #3474 is about. Bert added the test as XFAIL in r938071, and it was marked PASS by you, Philip, in r955334. So, I guess this wraps up that issue: can someone mark it as resolved? Could you mark it as resolved? Or don't you have the necessary permissions on tigris? Ok, done (didn't know I could do that myself :-)). There was a slight confusion when I read the test code, because a comment still talks about the tests as Currently this fails because The following patch removes that obsolete comment: [[[ Index: subversion/tests/cmdline/copy_tests.py === --- subversion/tests/cmdline/copy_tests.py (revision 1035851) +++ subversion/tests/cmdline/copy_tests.py (working copy) @@ -4358,8 +4358,6 @@ def copy_added_dir_with_copy(sbox): 'NewDir2/mu' : Item(status='A ', copied='+', wc_rev='-'), }) - # Currently this fails because NewDir2/mu loses its history in the copy - # from NewDir to NewDir2 svntest.actions.run_and_verify_status(wc_dir, expected_status) +1, but could you add a link to the issue (in a comment) while you're there? Committed in r1036306. Cheers, -- Johan
Re: svn commit: r1036078 - in /subversion/trunk/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h libsvn_fs_base/fs.c libsvn_fs_base/tree.c libsvn_fs_base/tree.h libsvn_fs_fs/fs
Mike, this generates a compiler warning (see at the end):
cmpil...@apache.org wrote on Wed, Nov 17, 2010 at 15:26:33 -:
Author: cmpilato
Date: Wed Nov 17 15:26:33 2010
New Revision: 1036078
URL: http://svn.apache.org/viewvc?rev=1036078view=rev
Log:
Fix a library dependency issue which was causing build failures. (As a
general rule, code inside the FS providers shouldn't call back into
the public FS API.) Sadly, the constraints of our system (namely the
BDB side of things) necessitate code duplication rather than sharing.
svn_error_t *
svn_fs_validate_mergeinfo(svn_mergeinfo_t *validated_mergeinfo,
- svn_mergeinfo_t mergeinfo,
svn_fs_t *fs,
+ svn_mergeinfo_t mergeinfo,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
@@ -197,6 +197,10 @@ typedef struct fs_vtable_t
svn_error_t *(*bdb_set_errcall)(svn_fs_t *fs,
void (*handler)(const char *errpfx,
char *msg));
+ svn_error_t *(*validate_mergeinfo)(svn_mergeinfo_t *validated_mergeinfo,
+ svn_mergeinfo_t mergeinfo,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
} fs_vtable_t;
This function doesn't have an svn_fs_t * parameter...
+/* Implements svn_fs_validate_mergeinfo. */
+svn_error_t *
+svn_fs_base__validate_mergeinfo(svn_mergeinfo_t *validated_mergeinfo,
+svn_fs_t *fs,
+svn_mergeinfo_t mergeinfo,
+apr_pool_t *result_pool,
+apr_pool_t *scratch_pool);
+
+/* Implements svn_fs_validate_mergeinfo. */
+svn_error_t *
+svn_fs_fs__validate_mergeinfo(svn_mergeinfo_t *validated_mergeinfo,
+ svn_fs_t *fs,
+ svn_mergeinfo_t mergeinfo,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
but these two do...
@@ -497,6 +497,7 @@ static fs_vtable_t fs_vtable = {
svn_fs_base__get_lock,
svn_fs_base__get_locks,
base_bdb_set_errcall,
+ svn_fs_base__validate_mergeinfo,
};
@@ -159,7 +159,8 @@ static fs_vtable_t fs_vtable = {
svn_fs_fs__unlock,
svn_fs_fs__get_lock,
svn_fs_fs__get_locks,
- fs_set_errcall
+ fs_set_errcall,
+ svn_fs_fs__validate_mergeinfo,
};
And therefore:
subversion/libsvn_fs_fs/fs.c:163: warning: initialization from incompatible
pointer type
subversion/libsvn_fs_base/fs.c:500: warning: initialization from incompatible
pointer type
RE: how to apply trademark symbols to Apache Subversion
Which trademarks are important here? * Apache(tm) * Subversion(r) * Apache Subversion ... ?? The TM and R symbols are appropriate for the first two, because they are by far the most important marks referred to on that web page. Note that, because the first two are ASF marks, therefore the combination of them is also our mark. We would never tolerate some other company putting Apache and Subversion together and creating their own mark for software! I hope, at the bottom of that web page you will say, in words: Apache and Apache Subversion are trademarks, and Subversion is a registered trademark, of The Apache Software Foundation. /Larry P.S. How close are we to our registration of the Apache mark? Only after that happens, we can change that TM to R, and modify the word notice -- if we then want to take the time to do so. /Larry -Original Message- From: Greg Stein [mailto:gst...@gmail.com] Sent: Wednesday, November 17, 2010 7:36 AM To: C. Michael Pilato Cc: legal-disc...@apache.org; tradema...@apache.org; dev@subversion.apache.org Subject: Re: how to apply trademark symbols to Apache Subversion On Wed, Nov 17, 2010 at 09:08, C. Michael Pilato cmpil...@collab.net wrote: On 11/17/2010 08:56 AM, Greg Stein wrote: Hi Legal peeps, The Subversion community updated the website to conform to the new guidelines, but we have a question on how to apply the appropriate trademark symbols. The guideline says to put a (tm) at the end of the first use of Apache Subversion, however, Subversion (alone) is a *registered* trademark. Should we just put an (r) after the first use of Subversion and leave it at that? And if that first use is Apache Subversion(r), would that still be fine? Maybe we should put an (r) after the first Apache Subversion and the first Subversion to clarify what is registered? I would expected that the common practice would be to, for the first use of Apache Subversion, display as Apache(tm) Subversion(r). Well, we have three terms/phrases in play here: * Apache(tm) * Subversion(r) * Apache Subversion ... ?? The first two are easy. But we also want to reserve Apache Subversion, much like we have Apache Tomcat(tm) (where the (tm) is implied for the two-word phrase) Cheers, -g - To unsubscribe, e-mail: legal-discuss-unsubscr...@apache.org For additional commands, e-mail: legal-discuss-h...@apache.org
[PATCH] Re: test failure 1.6.14 (svnsync test 29 with bdb; copyfrom)
Stefan Sperling wrote on Thu, Nov 18, 2010 at 00:54:42 +0100:
With BDB, svnsync crashes as follows during during svnsync test 29.
It only happens with BDB. Running ./svnsync_tests.py 29 --fs-type=bdb
triggers the problem for me on the 1.6.x branch, too.
I guess this could be related to this backport item:
* improve svnsync handling of dir copies (r962377, -8)
Details follow:
SVNProcessTerminatedBySignal
FAIL: svnsync_tests.py 29: descending into replaced dir looks in src
The following patch fixes the FAIL on trunk. (I haven't tested 1.6.x.)
Please review :-)
[[[
Follow-up to r962378 (for issue #3641):
When an API says COPYFROM_KNOWN might be false, believe that API and
implement a workaround for that case. Since this has almost doubled the
length of the copyfrom retrieval code, move it to a new static function.
This unbreaks svnsync_tests 29 under BDB.
* subversion/libsvn_repos/replay.c
(get_copyfrom): New helper.
(add_subdir): Delegate to get_copyfrom().
]]]
[[[
Index: subversion/libsvn_repos/replay.c
===
--- subversion/libsvn_repos/replay.c(revision 1036314)
+++ subversion/libsvn_repos/replay.c(working copy)
@@ -143,6 +143,36 @@ struct path_driver_cb_baton
apr_pool_t *pool;
};
+/* Helper: return in *COPYFROM_PATH_P and *COPYFROM_REV_P the copyfrom
+ * information; either from CHANGE, if available there, or those of
+ * COPYTO_ROOT:/COPYTO_PATH (which must be a copied path).
+ */
+static svn_error_t *
+get_copyfrom(const char **copyfrom_path_p,
+ svn_revnum_t *copyfrom_rev_p,
+ svn_fs_path_change2_t *change,
+ svn_fs_root_t *copyto_root,
+ const char *copyto_path,
+ apr_pool_t *pool)
+{
+ svn_fs_history_t *history, *history_prev;
+
+ /* Easy out. */
+ if (change-copyfrom_known)
+{
+ *copyfrom_path_p = change-copyfrom_path;
+ *copyfrom_rev_p = change-copyfrom_rev;
+ return SVN_NO_ERROR;
+}
+
+ SVN_ERR(svn_fs_node_history(history, copyto_root, copyto_path, pool));
+ SVN_ERR(svn_fs_history_prev(history_prev, history, TRUE /* cross copies */,
+ pool));
+ SVN_ERR(svn_fs_history_location(copyfrom_path_p, copyfrom_rev_p,
+ history_prev, pool));
+ return SVN_NO_ERROR;
+}
+
/* Recursively traverse PATH (as it exists under SOURCE_ROOT) emitting
the appropriate editor calls to add it and its children without any
history. This is meant to be used when either a subset of the tree
@@ -225,10 +255,10 @@ add_subdir(svn_fs_root_t *source_root,
continue;
else if (change-change_kind == svn_fs_path_change_replace)
{
- /* ### Can this assert fail? */
- SVN_ERR_ASSERT(change-copyfrom_known);
- copyfrom_path = change-copyfrom_path;
- copyfrom_rev = change-copyfrom_rev;
+ SVN_ERR(get_copyfrom(copyfrom_path, copyfrom_rev, change,
+ /* copied-to coordinates: */
+ target_root, new_path,
+ pool));
}
}
]]]
Thanks,
Daniel
Re: [PATCH] Re: test failure 1.6.14 (svnsync test 29 with bdb; copyfrom)
Daniel Shahaf wrote on Thu, Nov 18, 2010 at 06:07:07 +0200: + SVN_ERR(svn_fs_node_history(history, copyto_root, copyto_path, pool)); + SVN_ERR(svn_fs_history_prev(history_prev, history, TRUE /* cross copies */, + pool)); + SVN_ERR(svn_fs_history_location(copyfrom_path_p, copyfrom_rev_p, + history_prev, pool)); Tests running (trunk plus the patch). But perhaps I should have used this function instead: 1439 svn_error_t * 1440 svn_fs_copied_from(svn_revnum_t *rev_p, 1441const char **path_p, 1442svn_fs_root_t *root, 1443const char *path, 1444apr_pool_t *pool);
[PATCH] Fix for issue 3620 - switch command.
Log
[[[
Make 'svn switch' verify that the target path is local.
* subversion/libsvn_client/switch.c,
subversion/svn/switch-cmd.c
(svn_client_switch2, svn_cl__switch): Raise an error if target path is
not local.
* subversion/tests/cmdline/input_validation_tests.py
(invalid_switch_targets, test_list): New test.
Patch by: Noorul Islam K M noorul{_AT_}collab.net
]]]
Thanks and Regards
Noorul
Index: subversion/tests/cmdline/input_validation_tests.py
===
--- subversion/tests/cmdline/input_validation_tests.py (revision 1036324)
+++ subversion/tests/cmdline/input_validation_tests.py (working copy)
@@ -222,6 +222,12 @@
run_and_verify_svn_in_wc(sbox, svn:.*is not a local path, 'patch',
target1, target2)
+def invalid_switch_targets(sbox):
+ non-working copy paths for 'switch'
+ sbox.build(read_only=True)
+ run_and_verify_svn_in_wc(sbox, svn:.*is not a local path, 'switch',
+ ^/, ^/)
+
# Run the tests
@@ -247,6 +253,7 @@
invalid_unlock_targets,
invalid_status_targets,
invalid_patch_targets,
+ invalid_switch_targets,
]
if __name__ == '__main__':
Index: subversion/svn/switch-cmd.c
===
--- subversion/svn/switch-cmd.c (revision 1036324)
+++ subversion/svn/switch-cmd.c (working copy)
@@ -144,6 +144,13 @@
(SVN_ERR_BAD_URL, NULL,
_('%s' does not appear to be a URL), switch_url);
+ /* Target path cannot be URL */
+ if (svn_path_is_url(target))
+return svn_error_return(svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR,
+ NULL,
+ _('%s' is not a local path),
+ target));
+
/* Canonicalize the URL. */
switch_url = svn_uri_canonicalize(switch_url, scratch_pool);
Index: subversion/libsvn_client/switch.c
===
--- subversion/libsvn_client/switch.c (revision 1036324)
+++ subversion/libsvn_client/switch.c (working copy)
@@ -329,6 +329,11 @@
svn_client_ctx_t *ctx,
apr_pool_t *pool)
{
+ if (svn_path_is_url(path))
+return svn_error_return(svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+ _('%s' is not a local path),
+ path));
+
return svn_client__switch_internal(result_rev, path, switch_url,
peg_revision, revision, depth,
depth_is_sticky, NULL, ignore_externals,
[PATCH] Fix for issue 3620 - relocate command
Log
[[[
Make 'svn relocate' verify that the target working copy root dir is
local.
* subversion/libsvn_client/relocate.c,
subversion/svn/relocate-cmd.c
(svn_client_relocate2, svn_cl__relocate): Raise an error if target
working copy dir is not local.
* subversion/tests/cmdline/input_validation_tests.py
(invalid_relocate_targets, test_list): New test.
Patch by: Noorul Islam K M noorul{_AT_}collab.net
]]]
Thanks and Regards
Noorul
Index: subversion/tests/cmdline/input_validation_tests.py
===
--- subversion/tests/cmdline/input_validation_tests.py (revision 1036324)
+++ subversion/tests/cmdline/input_validation_tests.py (working copy)
@@ -222,6 +222,12 @@
run_and_verify_svn_in_wc(sbox, svn:.*is not a local path, 'patch',
target1, target2)
+def invalid_relocate_targets(sbox):
+ non-working copy paths for 'relocate'
+ sbox.build(read_only=True)
+ run_and_verify_svn_in_wc(sbox, svn:.*is not a local path, 'relocate',
+ ^/, ^/, ^/)
+
# Run the tests
@@ -247,6 +253,7 @@
invalid_unlock_targets,
invalid_status_targets,
invalid_patch_targets,
+ invalid_relocate_targets,
]
if __name__ == '__main__':
Index: subversion/svn/relocate-cmd.c
===
--- subversion/svn/relocate-cmd.c (revision 1036324)
+++ subversion/svn/relocate-cmd.c (working copy)
@@ -97,8 +97,20 @@
apr_pool_t *subpool = svn_pool_create(scratch_pool);
int i;
+ /* Target working copy root dir must be local. */
for (i = 2; i targets-nelts; i++)
{
+ path = APR_ARRAY_IDX(targets, i, const char *);
+ if (svn_path_is_url(path))
+return svn_error_return
+ (svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR,
+ NULL,
+ _('%s' is not a local path),
+ path));
+}
+
+ for (i = 2; i targets-nelts; i++)
+{
svn_pool_clear(subpool);
path = APR_ARRAY_IDX(targets, i, const char *);
SVN_ERR(svn_client_relocate2(path, from, to, ignore_externals,
Index: subversion/libsvn_client/relocate.c
===
--- subversion/libsvn_client/relocate.c (revision 1036324)
+++ subversion/libsvn_client/relocate.c (working copy)
@@ -217,6 +217,11 @@
vb.url_uuids = apr_array_make(pool, 1, sizeof(struct url_uuid_t));
vb.pool = pool;
+ if (svn_path_is_url(wcroot_dir))
+return svn_error_return(svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+ _('%s' is not a local path),
+ wcroot_dir));
+
SVN_ERR(svn_dirent_get_absolute(local_abspath, wcroot_dir, pool));
/* If we're ignoring externals, just relocate and get outta here. */
Re: Fix for issue 3620 - unlock command
Julian Foad julian.f...@wandisco.com writes: On Mon, 2010-11-15, Noorul Islam K M wrote: Noorul Islam K M noo...@collab.net writes: Noorul Islam K M noo...@collab.net writes: Make 'svn unlock' verify that both working copy paths and URLs are not passed. * subversion/tests/cmdline/input_validation_tests.py (invalid_unlock_targets): New test, verifying that svn unlock copes well with invalid target combinations. * subversion/svn/unlock-cmd.c (svn_cl__unlock): For consistency with other sub-commands, raise the SVN_ERR_CL_ARG_PARSING_ERROR if both working copy paths and URLs are passed, and use the same error message also used elsewhere. [...] Hi Noorul. I committed this as r1035208. Thanks again for this series of patches. Once this patch goes, I think we can wrap this issue. I tested by hand and found three more :-) svn patch foo ^/ svn relocate ^/ ^/ ^/ svn switch ^/ ^/ I mentioned them in the issue: http://subversion.tigris.org/issues/show_bug.cgi?id=3620. If you'd like to address those too, that would be wonderful. You don't have to, of course. Submitted patches for switch and relocate commands. Thanks and Regards Noorul
