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
Received on Wed Jul 11 09:40:01 2007