You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2007/07/11 07:40:27 UTC
[PATCH] Make 'svn commit --depth=foo' work.
This email contains:
1. A log message (surrounded by "[[[ ... ]]]")
2. A patch (also surrounded by "[[[ ... ]]]")
3. A shell script for testing the patch (also surrounded by "[[[ ... ]]]")
I'd love some review on this. It needs regression tests: what I plan
to do is convert that shell script over to our test harness. But for
getting this coded, it was easier to have a test script I could tweak
quickly.
This patch is in 'make check' right now. It passed basic_tests, but
got one failure in commit_tests:
commit_tests.py 26: commit named targets with -N (issues #1195, #1239)
(It's currently in prop_tests, but has encountered only that single
failure so far.)
I haven't yet had a chance to look at how serious the commit_tests #26
failure is. It might just be a case of "-N" behavior changing
slightly (i.e., we should change the test, not the code), or perhaps
it's a real bug in the code. I'll take a look at it later; right now
it's time for bed :-)
In the meantime, I wanted to send this so the change could get some
review.
-Karl
[[[
Make 'commit --depth=foo' work.
################################################################
### ###
### PATCH IN PROGRESS -- DO NOT COMMIT ###
### ###
### This seems to work, but I haven't finished running the ###
### regression tests yet, nor have I written new tests to ###
### cover this new functionality. Please don't commit it. ###
### ###
################################################################
* subversion/include/svn_client.h, subversion/libsvn_client/commit.c
(svn_client_commit4): Change recurse parameter to depth.
* subversion/libsvn_client/client.h
(svn_client__harvest_committables): Take depth instead of nonrecursive.
* subversion/libsvn_client/commit_util.c
(svn_client__harvest_committables): Take depth instead of nonrecursive.
(harvest_committables): Same, and remove a longstanding TODO with a
satisfying "*thwack*!".
(svn_client__get_copy_committables): Pass svn_depth_infinity,
not nonrecursive=FALSE, to harvest_committables.
* subversion/svn/commit-cmd.c
(svn_cl__commit): Pass opt_state->depth directly.
* notes/sparse-directories.txt: Remove item about svn_client_commit4.
]]]
[[[
Index: notes/sparse-directories.txt
===================================================================
--- notes/sparse-directories.txt (revision 25714)
+++ notes/sparse-directories.txt (working copy)
@@ -460,7 +460,6 @@
svn_client_import2() # Note: takes 'nonrecursive' right now
svn_client_revert() # Any point taking depth?
- svn_client_commit4() # Is manual control of depth needed here?
svn_client_propset3() # Is manual control of depth needed here?
svn_client_propget3() # Is manual control of depth needed here?
svn_client_resolved() # Is manual control of depth needed here?
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 25714)
+++ subversion/include/svn_client.h (working copy)
@@ -1512,15 +1512,17 @@
* @c svn_wc_notify_commit_deleted, @c svn_wc_notify_commit_replaced,
* @c svn_wc_notify_commit_postfix_txdelta.
*
- * ### TODO(sd): For consistency, this should probably take svn_depth_t
- * ### depth instead of svn_boolean_t recurse. But it's not needed
- * ### for the sparse-directories work right now, so leaving it alone
- * ### for now, although note that this is a 1.5 API so revving it
- * ### would be fairly painless.
+ * If @a depth is @c svn_depth_infinity, commit all changes to and
+ * below named targets. If @a depth is @c svn_depth_empty, commit
+ * only named targets (that is, only property changes on named
+ * directory targets, and property and content changes for named file
+ * targets). If @a depth is @c svn_depth_files, behave as above for
+ * named file targets, and for named directory targets, commit
+ * property changes on a named directory and all changes to files
+ * directly inside that directory. If @c svn_depth_immediates, behave
+ * as for @c svn_depth_files, and for subdirectories of any named
+ * directory target commit as though for @c svn_depth_empty.
*
- * If @a recurse is false, subdirectories of directories in @a targets
- * will be ignored.
- *
* Unlock paths in the repository, unless @a keep_locks is true.
*
* If @a changelist_name is non-NULL, then use it as a restrictive filter
@@ -1540,7 +1542,7 @@
svn_error_t *
svn_client_commit4(svn_commit_info_t **commit_info_p,
const apr_array_header_t *targets,
- svn_boolean_t recurse,
+ svn_depth_t depth,
svn_boolean_t keep_locks,
svn_boolean_t keep_changelist,
const char *changelist_name,
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 25714)
+++ subversion/libsvn_client/client.h (working copy)
@@ -697,8 +697,9 @@
locked in the process of this crawl. These will need to be
unlocked again post-commit.
- If NONRECURSIVE is specified, subdirectories of directory targets
- found in TARGETS will not be crawled for modifications.
+ If DEPTH is specified, descend (or not) into each target in TARGETS
+ as specified by DEPTH; the behavior is the same as that described
+ for svn_client_commit4().
If JUST_LOCKED is TRUE, treat unmodified items with lock tokens as
commit candidates.
@@ -715,7 +716,7 @@
apr_hash_t **lock_tokens,
svn_wc_adm_access_t *parent_dir,
apr_array_header_t *targets,
- svn_boolean_t nonrecursive,
+ svn_depth_t depth,
svn_boolean_t just_locked,
const char *changelist_name,
svn_client_ctx_t *ctx,
Index: subversion/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c (revision 25714)
+++ subversion/libsvn_client/commit_util.c (working copy)
@@ -186,9 +186,13 @@
entry ENTRY and ancestry URL), and add those candidates to
COMMITTABLES. If in ADDS_ONLY modes, only new additions are
recognized. COPYFROM_URL is the default copyfrom-url for children
- of copied directories. NONRECURSIVE indicates that this function
- will not recurse into subdirectories of PATH when PATH is itself a
- directory. Lock tokens of candidates will be added to LOCK_TOKENS, if
+ of copied directories.
+
+ DEPTH indicates how to treat files and subdirectories of PATH when
+ PATH is itself a directory; see svn_client__harvest_committables()
+ for its behavior.
+
+ Lock tokens of candidates will be added to LOCK_TOKENS, if
non-NULL. JUST_LOCKED indicates whether to treat non-modified items with
lock tokens as commit candidates.
@@ -216,7 +220,7 @@
const svn_wc_entry_t *parent_entry,
svn_boolean_t adds_only,
svn_boolean_t copy_mode,
- svn_boolean_t nonrecursive,
+ svn_depth_t depth,
svn_boolean_t just_locked,
const char *changelist_name,
svn_client_ctx_t *ctx,
@@ -505,13 +509,10 @@
}
}
- /* For directories, recursively handle each of their entries (except
- when the directory is being deleted, unless the deletion is part
- of a replacement ... how confusing). Oh, and don't recurse at
- all if this is a nonrecursive commit. ### We'll probably make
- the whole 'nonrecursive' concept go away soon and be replaced
- with the more sophisticated Depth0|Depth1|DepthInfinity. */
- if (entries && (! nonrecursive)
+ /* For directories, recursively handle each entry according to depth
+ (except when the directory is being deleted, unless the deletion
+ is part of a replacement ... how confusing). */
+ if (entries && (depth > svn_depth_empty)
&& ((! (state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE))
|| (state_flags & SVN_CLIENT_COMMIT_ITEM_ADD)))
{
@@ -561,64 +562,94 @@
/* Recurse. */
if (this_entry->kind == svn_node_dir)
{
- svn_error_t *lockerr;
- lockerr = svn_wc_adm_retrieve(&dir_access, adm_access,
- full_path, loop_pool);
-
- if (lockerr)
+ if (depth <= svn_depth_files)
{
- if (lockerr->apr_err == SVN_ERR_WC_NOT_LOCKED)
+ /* Don't bother with any of this if it's a directory
+ and depth says not to go there. */
+ continue;
+ }
+ else
+ {
+ svn_error_t *lockerr;
+ lockerr = svn_wc_adm_retrieve(&dir_access, adm_access,
+ full_path, loop_pool);
+
+ if (lockerr)
{
- /* A missing, schedule-delete child dir is
- allowable. Just don't try to recurse. */
- svn_node_kind_t childkind;
- svn_error_t *err = svn_io_check_path(full_path,
- &childkind,
- loop_pool);
- if (! err
- && childkind == svn_node_none
- && this_entry->schedule == svn_wc_schedule_delete)
+ if (lockerr->apr_err == SVN_ERR_WC_NOT_LOCKED)
{
- if ((changelist_name == NULL)
- || (entry->changelist
- && (strcmp(changelist_name,
- entry->changelist) == 0)))
+ /* A missing, schedule-delete child dir is
+ allowable. Just don't try to recurse. */
+ svn_node_kind_t childkind;
+ svn_error_t *err = svn_io_check_path(full_path,
+ &childkind,
+ loop_pool);
+ if (! err
+ && (childkind == svn_node_none)
+ && (this_entry->schedule
+ == svn_wc_schedule_delete))
{
- add_committable(committables, full_path,
- this_entry->kind, used_url,
- SVN_INVALID_REVNUM,
- NULL,
- SVN_INVALID_REVNUM,
- SVN_CLIENT_COMMIT_ITEM_DELETE);
- svn_error_clear(lockerr);
- continue; /* don't recurse! */
+ if ((changelist_name == NULL)
+ || (entry->changelist
+ && (strcmp(changelist_name,
+ entry->changelist) == 0)))
+ {
+ add_committable(
+ committables, full_path,
+ this_entry->kind, used_url,
+ SVN_INVALID_REVNUM,
+ NULL,
+ SVN_INVALID_REVNUM,
+ SVN_CLIENT_COMMIT_ITEM_DELETE);
+ svn_error_clear(lockerr);
+ continue; /* don't recurse! */
+ }
}
+ else
+ {
+ svn_error_clear(err);
+ return lockerr;
+ }
}
else
- {
- svn_error_clear(err);
- return lockerr;
- }
+ return lockerr;
}
- else
- return lockerr;
}
}
else
- dir_access = adm_access;
+ {
+ dir_access = adm_access;
+ }
- SVN_ERR(harvest_committables
- (committables, lock_tokens, full_path, dir_access,
- used_url ? used_url : this_entry->url,
- this_cf_url,
- this_entry,
- entry,
- adds_only,
- copy_mode,
- FALSE, just_locked,
- changelist_name,
- ctx,
- loop_pool));
+ {
+ svn_depth_t depth_below_here = depth;
+
+ /* If depth is svn_depth_files, then we must be recursing
+ into a file, or else we wouldn't be here -- either way,
+ svn_depth_empty is the right depth to use. On the
+ other hand, if depth is svn_depth_immediates, then we
+ could be recursing into a directory or a file -- in
+ which case svn_depth_empty is *still* the right depth
+ to use. I know that sounds bizarre (one normally
+ expects to just do depth - 1) but it's correct. */
+ if (depth == svn_depth_immediates
+ || depth == svn_depth_files)
+ depth_below_here = svn_depth_empty;
+
+ SVN_ERR(harvest_committables
+ (committables, lock_tokens, full_path, dir_access,
+ used_url ? used_url : this_entry->url,
+ this_cf_url,
+ this_entry,
+ entry,
+ adds_only,
+ copy_mode,
+ depth_below_here,
+ just_locked,
+ changelist_name,
+ ctx,
+ loop_pool));
+ }
}
svn_pool_destroy(loop_pool);
@@ -642,7 +673,7 @@
apr_hash_t **lock_tokens,
svn_wc_adm_access_t *parent_dir,
apr_array_header_t *targets,
- svn_boolean_t nonrecursive,
+ svn_depth_t depth,
svn_boolean_t just_locked,
const char *changelist_name,
svn_client_ctx_t *ctx,
@@ -772,7 +803,7 @@
subpool));
SVN_ERR(harvest_committables(*committables, *lock_tokens, target,
dir_access, entry->url, NULL,
- entry, NULL, FALSE, FALSE, nonrecursive,
+ entry, NULL, FALSE, FALSE, depth,
just_locked, changelist_name,
ctx, subpool));
@@ -857,8 +888,8 @@
allocate the new commit_item, we can safely use the iterpool here. */
SVN_ERR(harvest_committables(*committables, NULL, pair->src,
dir_access, pair->dst, entry->url, entry,
- NULL, FALSE, TRUE, FALSE, FALSE,
- NULL, ctx, iterpool));
+ NULL, FALSE, TRUE, svn_depth_infinity,
+ FALSE, NULL, ctx, iterpool));
}
svn_pool_destroy(iterpool);
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 25714)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -1156,7 +1156,7 @@
svn_error_t *
svn_client_commit4(svn_commit_info_t **commit_info_p,
const apr_array_header_t *targets,
- svn_boolean_t recurse,
+ svn_depth_t depth,
svn_boolean_t keep_locks,
svn_boolean_t keep_changelist,
const char *changelist_name,
@@ -1193,9 +1193,23 @@
_("'%s' is a URL, but URLs cannot be commit targets"), target);
}
- /* Condense the target list. */
+ /* Condense the target list.
+
+ ### TODO(sd): Back when svn_path_condense_targets() was written,
+ ### there was no depth, only a recurse boolean, so condensation
+ ### was a yes-no proposition.
+ ###
+ ### Nowadays things are a little more complex. For example, if
+ ### depth is svn_depth_files, and two targets are "foo" and
+ ### "foo/bar", we should ideally condense out the latter if
+ ### "foo/bar" is a file but not if it is a directory.
+ ###
+ ### However, the code isn't that smart yet, which might mean
+ ### we'll grab more/deeper recursive adm locks than we really
+ ### need.
+ */
SVN_ERR(svn_path_condense_targets(&base_dir, &rel_targets, targets,
- recurse, pool));
+ depth == svn_depth_infinity, pool));
/* No targets means nothing to commit, so just return. */
if (! base_dir)
@@ -1235,7 +1249,7 @@
/* If the final target is a dir, we want to recursively lock it */
if (kind == svn_node_dir)
{
- if (recurse)
+ if (depth == svn_depth_infinity || depth == svn_depth_immediates)
APR_ARRAY_PUSH(dirs_to_lock_recursive, const char *) = target;
else
APR_ARRAY_PUSH(dirs_to_lock, const char *) = target;
@@ -1271,7 +1285,7 @@
/* If the final target is a dir, we want to lock it */
if (kind == svn_node_dir)
{
- if (recurse)
+ if (depth == svn_depth_infinity || depth == svn_depth_immediates)
APR_ARRAY_PUSH(dirs_to_lock_recursive,
const char *) = apr_pstrdup(pool, target);
else
@@ -1391,7 +1405,25 @@
target, pool),
_("Are all the targets part of the same working copy?"));
- if (!recurse)
+ /* ### TODO(sd): This check is slightly too strict. It should be
+ ### possible to:
+ ###
+ ### * delete an empty directory when depth==svn_depth_empty;
+ ###
+ ### * delete a directory containing only files when
+ ### depth==svn_depth_files;
+ ###
+ ### * delete a directory containing only files and empty
+ ### subdirs when depth==svn_depth_immediates.
+ ###
+ ### But for now, we insist on svn_depth_infinity if you're
+ ### going to delete a directory, because we're lazy and
+ ### trying to get depthy commits working in the first place.
+ ###
+ ### This would be fairly easy to fix, though: just, well,
+ ### check the above conditions!
+ */
+ if (depth != svn_depth_infinity)
{
svn_wc_status2_t *status;
svn_node_kind_t kind;
@@ -1415,7 +1447,7 @@
&lock_tokens,
base_dir_access,
rel_targets,
- recurse ? FALSE : TRUE,
+ depth,
! keep_locks,
changelist_name,
ctx,
Index: subversion/svn/commit-cmd.c
===================================================================
--- subversion/svn/commit-cmd.c (revision 25714)
+++ subversion/svn/commit-cmd.c (working copy)
@@ -100,7 +100,7 @@
(ctx->log_msg_baton3,
svn_client_commit4(&commit_info,
targets,
- SVN_DEPTH_TO_RECURSE(opt_state->depth),
+ opt_state->depth,
no_unlock,
opt_state->keep_changelist,
opt_state->changelist,
]]]
[[[
#!/bin/sh
# Put this script in its own directory and run it. If you set the
# line below correctly, everything else should work automatically.
# Change this to wherever your Subversion source build lives:
BASE_DIR=/home/kfogel/src/subversion
# You shouldn't have to change anything below this line, but if you
# want to test over http:// instead of svn://, you could choose a
# different URL here.
URL=svn://localhost/repos
# URL=http://localhost/SOMETHING/repos
# URL=file:///`pwd`/repos
SVN=${BASE_DIR}/subversion/svn/svn
SVNSERVE=${BASE_DIR}/subversion/svnserve/svnserve
SVNADMIN=${BASE_DIR}/subversion/svnadmin/svnadmin
rm -rf import-me repos wc
${SVNADMIN} create repos
echo "[general]" > repos/conf/svnserve.conf
echo "anon-access = write" >> repos/conf/svnserve.conf
echo "auth-access = write" >> repos/conf/svnserve.conf
${SVNSERVE} --pid-file svnserve-pid -d -r `pwd`
echo "### Making a Greek Tree for import..."
mkdir import-me
mkdir import-me/A
mkdir import-me/A/B/
mkdir import-me/A/C/
mkdir import-me/A/D/
mkdir import-me/A/B/E/
mkdir import-me/A/B/F/
mkdir import-me/A/D/G/
mkdir import-me/A/D/H/
echo "This is the file 'iota'." > import-me/iota
echo "This is the file 'A/mu'." > import-me/A/mu
echo "This is the file 'A/B/lambda'." > import-me/A/B/lambda
echo "This is the file 'A/B/E/alpha'." > import-me/A/B/E/alpha
echo "This is the file 'A/B/E/beta'." > import-me/A/B/E/beta
echo "This is the file 'A/D/gamma'." > import-me/A/D/gamma
echo "This is the file 'A/D/G/pi'." > import-me/A/D/G/pi
echo "This is the file 'A/D/G/rho'." > import-me/A/D/G/rho
echo "This is the file 'A/D/G/tau'." > import-me/A/D/G/tau
echo "This is the file 'A/D/H/chi'." > import-me/A/D/H/chi
echo "This is the file 'A/D/H/omega'." > import-me/A/D/H/omega
echo "This is the file 'A/D/H/psi'." > import-me/A/D/H/psi
echo "### Done."
echo ""
echo "### Importing it..."
(cd import-me; ${SVN} import -q -m "Initial import." ${URL})
echo "### Done."
echo ""
echo "### Test that depth-upgrading a working copy works as expected. ###"
echo ""
echo "### Checking out a depth-files working copy..."
${SVN} co --depth=files -q ${URL}/ wc
echo "### Done."
echo ""
echo "### Listing the working copy:"
ls -R wc
echo ""
echo "### Now updating it to depth-infinity..."
(cd wc; ${SVN} up -q --depth=infinity)
echo "### Done."
echo ""
echo "### Listing the working copy:"
ls -R wc
echo ""
echo "### If you got just 'iota' in the first listing above, and a"
echo "### full Greek Tree in the second listing, then upgrading from"
echo "### depth-files to depth-infinity worked as expected."
echo ""
# Set up some changes.
${SVN} propset -q foo bar wc/A/D
${SVN} propset -q color blue wc/A/D/H
echo "This is a change to A/D/gamma." >> wc/A/D/gamma
echo "This is a change to A/D/G/pi." >> wc/A/D/G/pi
echo "This is a change to A/D/H/omega." >> wc/A/D/H/omega
echo "### All the commit tests will take place inside wc/A/D/. ###"
echo ""
# Commit with --depth=empty.
cd wc
echo "### 'svn status -q' in the working copy before any commits:"
${SVN} st -q
echo ""
echo "### There should be three modified files and two propchanges above."
echo ""
# Commit with --depth=empty.
echo "### Test that committing --depth=empty works as expected. ###"
echo ""
cd A/D
echo "### Committing with --depth=empty from inside A/D/."
${SVN} commit -m 'Commit just the propchange to D.' --depth=empty
echo "### Done."
echo ""
cd ../..
# Test that --depth=empty worked as expected.
echo "### 'svn status -q' in the working copy after the commit:"
${SVN} st -q
echo ""
echo "### There should be three modified files and one propchange above,"
echo "### else 'svn commit --depth=empty' isn't working right."
echo ""
# Make there be a propchange on D again, since it should be committed
# with every deeper depth as well.
${SVN} propset -q foo baz A/D
# Commit with --depth=files.
echo "### Test that committing --depth=files works as expected. ###"
echo ""
cd A/D
echo "### Committing with --depth=files from inside A/D/."
${SVN} commit -m 'Commit one propchange and gamma.' --depth=files
echo "### Done."
echo ""
cd ../..
# Test that --depth=files worked as expected.
echo "### 'svn status -q' in the working copy after the commit:"
${SVN} st -q
echo ""
echo "### There should be two modified files and one propchange above,"
echo "### else 'svn commit --depth=files' isn't working right."
echo ""
# Restore propchange again and file mod.
${SVN} propset -q foo qux A/D
echo "This is another change to A/D/gamma." >> A/D/gamma
# Commit with --depth=immediates.
echo "### Test that committing --depth=immediates works as expected. ###"
echo ""
cd A/D
echo "### Committing with --depth=immediates from inside A/D/."
${SVN} commit -m 'Commit two propchanges and gamma.' --depth=immediates
echo "### Done."
echo ""
cd ../..
# Test that --depth=immediates worked as expected.
echo "### 'svn status -q' in the working copy after the commit:"
${SVN} st -q
echo ""
echo "### There should be two modified files (in G and H) above, else"
echo "### 'svn commit --depth=immediates' isn't working right."
echo ""
# Restore propchanges and file mod.
${SVN} propset -q foo quux A/D
${SVN} propset -q color green A/D/H
echo "This is another change to A/D/gamma." >> A/D/gamma
# Commit with --depth=infinity.
echo "### Test that committing --depth=infinity works as expected. ###"
echo "### "
cd A/D
echo "Committing with --depth=infinity from inside A/D/."
${SVN} commit -m 'Commit two propchanges and three files.' --depth=infinity
echo "### Done."
echo ""
cd ../..
# Test that --depth=infinity worked as expected.
echo "### 'svn status -q' in the working copy after the commit:"
${SVN} st -q
echo ""
echo "### There should be no modifications above, else"
echo "### 'svn commit --depth=infinity' isn't working right."
echo ""
# Return to parent of wc.
cd ..
# Shut down the server.
kill -9 `cat svnserve-pid`
]]]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Make 'svn commit --depth=foo' work.
Posted by Karl Fogel <kf...@red-bean.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> But before even reading the patch or trying your test script, I was anxious
> to see how you would handle what I suspected would be a little booger of a
> programming challenge, namely dealing with the situation where you have two
> changes that are more than one depth level apart, yet individually specified
> on the 'svn commit' command-line with different depth levels:
>
> $ svn st
> M A/D/G/rho
> M iota
>
> Now, I try 'svn ci' with various depths that I think should catch both
> changes. The following two combinations don't work:
>
> $ svn ci --depth immediates . A/D/G
> subversion/libsvn_wc/lock.c:561: (apr_err=155004)
> svn: Working copy '/home/cmpilato/tests/karl-patch/wc' locked
> svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
> $ svn ci --depth immediates . A/D/G/rho
> subversion/libsvn_wc/lock.c:561: (apr_err=155004)
> svn: Working copy '/home/cmpilato/tests/karl-patch/wc' locked
> svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
Fixed in r26487.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Make 'svn commit --depth=foo' work.
Posted by "C. Michael Pilato" <cm...@collab.net>.
Karl Fogel wrote:
> This email contains:
>
> 1. A log message (surrounded by "[[[ ... ]]]")
> 2. A patch (also surrounded by "[[[ ... ]]]")
> 3. A shell script for testing the patch (also surrounded by "[[[ ... ]]]")
>
> I'd love some review on this. It needs regression tests: what I plan
> to do is convert that shell script over to our test harness. But for
> getting this coded, it was easier to have a test script I could tweak
> quickly.
>
> This patch is in 'make check' right now. It passed basic_tests, but
> got one failure in commit_tests:
>
> commit_tests.py 26: commit named targets with -N (issues #1195, #1239)
>
> (It's currently in prop_tests, but has encountered only that single
> failure so far.)
>
> I haven't yet had a chance to look at how serious the commit_tests #26
> failure is. It might just be a case of "-N" behavior changing
> slightly (i.e., we should change the test, not the code), or perhaps
> it's a real bug in the code. I'll take a look at it later; right now
> it's time for bed :-)
>
> In the meantime, I wanted to send this so the change could get some
> review.
I've taken a glance at the patch, and I think that the changes you've made
look pretty good, at least in terms of the way the compiler and test
environment in my head evaluates them.
But before even reading the patch or trying your test script, I was anxious
to see how you would handle what I suspected would be a little booger of a
programming challenge, namely dealing with the situation where you have two
changes that are more than one depth level apart, yet individually specified
on the 'svn commit' command-line with different depth levels:
$ svn st
M A/D/G/rho
M iota
Now, I try 'svn ci' with various depths that I think should catch both
changes. The following two combinations don't work:
$ svn ci --depth immediates . A/D/G
subversion/libsvn_wc/lock.c:561: (apr_err=155004)
svn: Working copy '/home/cmpilato/tests/karl-patch/wc' locked
svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
$ svn ci --depth immediates . A/D/G/rho
subversion/libsvn_wc/lock.c:561: (apr_err=155004)
svn: Working copy '/home/cmpilato/tests/karl-patch/wc' locked
svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
These seem to work, though:
$ svn ci --depth files . A/D/G
$ svn ci --depth files . A/D/G/rho
$ svn ci --depth empty . A/D/G/rho
I wasn't able to quickly find the cause, though.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand