[[[ Working towards issues #3217: svn commit should show unknown files in editor Create a new version of the commit message callback so that we can pass an array of unversioned items. This array can be used by a client to display a list of unversioned items along with the items to be committed, when invoking an external editor. Modify harvest_committables to look for unversioned files while recursing over the working copy. This is done with the new helper function find_committables. * subversion/include/svn_client.h (svn_client_get_commit_log4_t): New callback with array of unversioned items. (svn_client_get_commit_log3_t): Deprecated. (svn_client_ctx_t): Added log_msg_func4 and log_msg_baton4. * subversion/libsvn_client/commit_util.c (svn_client__get_log_msg): Call new svn_client_get_commit_log4_t if provided by ctx, else revert the to old api. Update all callers. (svn_client__harvest_committables): Add unversioned_items hash to the parameter list and make neccessary changes for the modified harvest_committables. (harvest_committables): Add unversioned_items and directory_cache to the parameter list. Call find_unversioned_items on each directory we scan as well as the parent directory of each item we add to committables. (find_unversioned_items): New helper function. This is called from within harvest_committables to compare the children of the given path to its versioned entries. Any unversioned items are added to unversioned_items. (harvest_copy_committables): Update call to harvest_committables. * subversion/libsvn_client/client.h (SVN_CLIENT__HAS_LOG_MSG_FUNC): Add a condition for our new log message callback. (svn_client__get_log_msg): Add the new parameter apr_array_header_t *unversioned_items, that is used to pass unversioned item list to the callback. (svn_client__harvest_committables): Add unversioned_items, a hash used to collect unversioned items in while recursing the working copy. * subversion/libsvn_client/commit.c (svn_client_commit4): Use the updated svn_client__harvest_committables to to pass any unversioned items to svn_client__get_log_msg. ]]] Index: subversion/include/svn_client.h =================================================================== --- subversion/include/svn_client.h (revision 33218) +++ subversion/include/svn_client.h (working copy) @@ -577,11 +577,33 @@ * structures, which may be fully or only partially filled-in, * depending on the type of commit operation. * + * @a unversioned_items is a read-only array of const char * strings + * that are the paths of any unversioned items that are currently + * contained in the same directories of all other commit targets. + * Ignored items are not returned. For commit operations that involve + * direct repository URLs, unversioned_items will be @c NULL. + * * @a baton is provided along with the callback for use by the handler. * * All allocations should be performed in @a pool. * + * @since New in 1.6. + */ +typedef svn_error_t *(*svn_client_get_commit_log4_t) + (const char **log_msg, + const char **tmp_file, + const apr_array_header_t *commit_items, + const apr_array_header_t *unversioned_items, + void *baton, + apr_pool_t *pool); + + +/** Similar to svn_client_get_commit_log4_t but without + * unversioned_items. + * * @since New in 1.5. + * + * @deprecated Provided for backward compatibility with 1.5 the API. */ typedef svn_error_t *(*svn_client_get_commit_log3_t) (const char **log_msg, @@ -895,6 +917,16 @@ * @since New in 1.5. */ const char *client_name; + /** Log message callback function. NULL means that Subversion + * should try @c log_msg_func3, then @c log_msg_func2 and + * @c log_msg_func + * @since New in 1.6. */ + svn_client_get_commit_log4_t log_msg_func4; + + /** The callback baton for @c log_msg_func4. + * @since New in 1.6. */ + void *log_msg_baton4; + } svn_client_ctx_t; /** Initialize a client context. Index: subversion/libsvn_client/delete.c =================================================================== --- subversion/libsvn_client/delete.c (revision 33218) +++ subversion/libsvn_client/delete.c (working copy) @@ -162,7 +162,7 @@ APR_ARRAY_PUSH(commit_items, svn_client_commit_item3_t *) = item; } SVN_ERR(svn_client__get_log_msg(&log_msg, &tmp_file, commit_items, - ctx, pool)); + NULL, ctx, pool)); if (! log_msg) { svn_pool_destroy(subpool); Index: subversion/libsvn_client/client.h =================================================================== --- subversion/libsvn_client/client.h (revision 33218) +++ subversion/libsvn_client/client.h (working copy) @@ -348,7 +348,10 @@ /* CTX is of type "svn_client_ctx_t *". */ #define SVN_CLIENT__HAS_LOG_MSG_FUNC(ctx) \ - ((ctx)->log_msg_func3 || (ctx)->log_msg_func2 || (ctx)->log_msg_func) + ((ctx)->log_msg_func4 \ + || (ctx)->log_msg_func3 \ + || (ctx)->log_msg_func2 \ + || (ctx)->log_msg_func) /* This is the baton that we pass svn_ra_open3(), and is associated with the callback table we provide to RA. */ @@ -838,6 +841,11 @@ when harvesting committables; that is, don't add a path to COMMITTABLES unless it's a member of one of those changelists. + UNVERSIONED_ITEMS should either be an apr_hash_t * or set to NULL. + When non-NULL, any unversioned items that are found while searching + for committable items are added. Otherwise unversioned items are + ignored. + If CTX->CANCEL_FUNC is non-null, it will be called with CTX->CANCEL_BATON while harvesting to determine if the client has cancelled the operation. */ @@ -849,6 +857,7 @@ svn_depth_t depth, svn_boolean_t just_locked, const apr_array_header_t *changelists, + apr_hash_t *unversioned_items, svn_client_ctx_t *ctx, apr_pool_t *pool); @@ -1013,13 +1022,14 @@ /* Retrieve log messages using the first provided (non-NULL) callback - in the set of *CTX->log_msg_func3, CTX->log_msg_func2, or - CTX->log_msg_func. Other arguments same as + in the set of *CTX->log_msg_func4, *CTX->log_msg_func3, + CTX->log_msg_func2, or CTX->log_msg_func. Other arguments same as svn_client_get_commit_log3_t. */ svn_error_t * svn_client__get_log_msg(const char **log_msg, const char **tmp_file, const apr_array_header_t *commit_items, + const apr_array_header_t *unversioned_items, svn_client_ctx_t *ctx, apr_pool_t *pool); Index: subversion/libsvn_client/prop_commands.c =================================================================== --- subversion/libsvn_client/prop_commands.c (revision 33218) +++ subversion/libsvn_client/prop_commands.c (working copy) @@ -280,7 +280,7 @@ item->state_flags = SVN_CLIENT_COMMIT_ITEM_PROP_MODS; APR_ARRAY_PUSH(commit_items, svn_client_commit_item3_t *) = item; SVN_ERR(svn_client__get_log_msg(&message, &tmp_file, commit_items, - ctx, pool)); + NULL, ctx, pool)); if (! message) return SVN_NO_ERROR; } Index: subversion/libsvn_client/copy.c =================================================================== --- subversion/libsvn_client/copy.c (revision 33218) +++ subversion/libsvn_client/copy.c (working copy) @@ -978,7 +978,7 @@ } SVN_ERR(svn_client__get_log_msg(&message, &tmp_file, commit_items, - ctx, pool)); + NULL, ctx, pool)); if (! message) return SVN_NO_ERROR; } @@ -1205,7 +1205,7 @@ } SVN_ERR(svn_client__get_log_msg(&message, &tmp_file, commit_items, - ctx, pool)); + NULL, ctx, pool)); if (! message) return svn_wc_adm_close(adm_access); } Index: subversion/libsvn_client/commit_util.c =================================================================== --- subversion/libsvn_client/commit_util.c (revision 33218) +++ subversion/libsvn_client/commit_util.c (working copy) @@ -23,6 +23,7 @@ #include #include +#include #include "client.h" #include "svn_path.h" @@ -32,6 +33,7 @@ #include "svn_md5.h" #include "svn_iter.h" #include "svn_hash.h" +#include "svn_io.h" #include #include /* for qsort() */ @@ -185,6 +187,168 @@ svn_client__default_walker_error_handler }; +/* Look for unversioned items in PATH and add any that are found + to the UNVERSIONED_ITEMS hash. If UNVERSIONED_ITEMS is NULL, just + return SVN_NO_ERROR and do nothing. + + ENTRIES should be a hash like that returned by svn_wc_entries_read, + corresponding to PATH. If ENTRIES is NULL, ADM_ACCESS is used + to populate ENTRIES for PATH. + + ADM_ACCESS must be set to the admin baton for path and is used + to collect info about ignored files, externals and when ENTRIES + is NULL. + + DIRECTORY_CACHE should point to a hash that persists between calls. + It is used internally to check we only search a given directory once. */ + +static svn_error_t * +find_unversioned_items(apr_hash_t *unversioned_items, + apr_hash_t *entries, + const char *path, + svn_wc_adm_access_t *adm_access, + svn_boolean_t copy_mode, + apr_hash_t *directory_cache, + svn_client_ctx_t *ctx, + apr_pool_t *pool) +{ + apr_hash_index_t *hi; + apr_hash_t *dirents; + apr_hash_t *externals; + apr_pool_t *subpool = svn_pool_create(pool); + apr_pool_t *iterpool; + apr_pool_t *hashpool; + const char *repos_name = SVN_CLIENT__SINGLE_REPOS_NAME; + const char *checked_dir = NULL; + apr_array_header_t *array; + apr_array_header_t *patterns; + + /* If there is no UNVERSIONED_ITEMS hash, then the caller doesn't + want to look for unversioned items, so we just return. */ + if (! unversioned_items) + return SVN_NO_ERROR; + + /* If this path is cached don't re-check, just return now. */ + if (apr_hash_get(directory_cache, path, APR_HASH_KEY_STRING)) + return SVN_NO_ERROR; + + SVN_ERR(svn_wc_get_ignores(&patterns, ctx->config, adm_access, subpool)); + + /* If ENTRIES is NULL, then we need to use adm_access to + get an admin baton for PATH and use it to find the versioned + items from PATH. */ + if (! entries) + { + svn_wc_adm_access_t *dir_access; + + /* If the root of the working copy gets added to committables + then PATH will be passed as the roots parent directory. When we + try to open the admin area it will obviously fail. Is there a + better way to work out if PATH is the root instead of just + catching the error and then clearing it? */ + svn_error_t *err = svn_wc_adm_retrieve(&dir_access, adm_access, + path, subpool); + + /* If we can't get an admin baton for PATH we just bail out. */ + if (err) + { + svn_error_clear(err); + return SVN_NO_ERROR; + } + else + { + err = svn_wc_entries_read(&entries, dir_access, + copy_mode, pool); + if (err) + { + svn_error_clear(err); + return SVN_NO_ERROR; + } + + } + } + + /* Find any externals inside path, so we can ignore them */ + externals = apr_hash_make(subpool); + { + const svn_string_t *prop_val; + + SVN_ERR(svn_wc_prop_get(&prop_val, SVN_PROP_EXTERNALS, path, + adm_access, subpool)); + if (prop_val) + { + apr_array_header_t *ext_items; + int i; + + SVN_ERR(svn_wc_parse_externals_description3(&ext_items, path, + prop_val->data, FALSE, + pool)); + + for (i = 0; ext_items && i < ext_items->nelts; i++) + { + svn_wc_external_item2_t *item; + + item = APR_ARRAY_IDX(ext_items, i, svn_wc_external_item2_t *); + apr_hash_set(externals, apr_pstrdup(subpool, + item->target_dir), + APR_HASH_KEY_STRING, item); + } + } + } + + /* Now we can start to look for unversioned items in path. */ + hashpool = apr_hash_pool_get(unversioned_items); + + /* Read PATH's dirents. */ + SVN_ERR(svn_io_get_dir_filenames(&dirents, path, subpool)); + + array = apr_hash_get(unversioned_items, repos_name, + APR_HASH_KEY_STRING); + if (! array) + { + array = apr_array_make(hashpool, 1, sizeof(char *)); + apr_hash_set(unversioned_items, repos_name, APR_HASH_KEY_STRING, + array); + } + + iterpool = svn_pool_create(subpool); + for (hi = apr_hash_first(subpool, dirents); hi; + hi = apr_hash_next(hi)) + { + const void *key; + const void *entries_key; + apr_ssize_t klen; + void *val; + + svn_pool_clear(iterpool); + + apr_hash_this(hi, &key, &klen, &val); + + /* Skip versioned entries, externals, any ignored items and the + administrative directory. */ + if (apr_hash_get(entries, key, klen) + || apr_hash_get(externals, key, klen) + || svn_wc_is_adm_dir(key, iterpool) + || svn_wc_match_ignore_list(key, patterns, iterpool)) + continue; + + /* If we get here, then key is an unversioned item in the current + path that we need to add to our list */ + APR_ARRAY_PUSH(array, char *) = svn_path_join(path, key, hashpool); + } + apr_pool_destroy(iterpool); + + /* Add the directory we just checked to our cache so we don't + have to check it again */ + checked_dir = apr_pstrdup(apr_hash_pool_get(unversioned_items), path); + apr_hash_set(directory_cache, checked_dir, APR_HASH_KEY_STRING, + checked_dir); + + svn_pool_destroy(subpool); + + return SVN_NO_ERROR; +} + /* Recursively search for commit candidates in (and under) PATH (with entry ENTRY and ancestry URL), and add those candidates to COMMITTABLES. If in ADDS_ONLY modes, only new additions are @@ -208,6 +372,13 @@ when harvesting committables; that is, don't add a path to COMMITTABLES unless it's a member of one of those changelists. + Add any unversioned items that are discovered to UNVERSIONED_ITEMS. + UNVERSIONED_ITEMS may be passed as NULL by callers that aren't + interested in unversioned items. DIRECTORY_CACHE is passed to + find_unversioned_items where it is used internally. It should be NULL + when harvest_committables is first called and should persist between + calls to harvest_committables. + If CTX->CANCEL_FUNC is non-null, call it with CTX->CANCEL_BATON to see if the user has cancelled the operation. @@ -227,6 +398,8 @@ svn_depth_t depth, svn_boolean_t just_locked, apr_hash_t *changelists, + apr_hash_t *unversioned_items, + apr_hash_t *directory_cache, svn_client_ctx_t *ctx, apr_pool_t *pool) { @@ -247,6 +420,12 @@ if (look_up_committable(committables, path, pool)) return SVN_NO_ERROR; + /* if we have a unversioned_items hash, check to see if the directory + cache has been created. If not - do so. */ + if (unversioned_items) + if (! directory_cache) + directory_cache = apr_hash_make(pool); + SVN_ERR_ASSERT(entry); SVN_ERR_ASSERT(url); @@ -329,6 +508,9 @@ SVN_ERR(svn_wc_conflicted_p2(&tc, &pc, &treec, p_path, entry, pool)); } + + SVN_ERR(find_unversioned_items(unversioned_items, entries, path, adm_access, + copy_mode, directory_cache, ctx, pool)); } /* If this is not a directory, check for conflicts using the @@ -534,6 +716,14 @@ apr_hash_set(lock_tokens, apr_pstrdup(token_pool, url), APR_HASH_KEY_STRING, apr_pstrdup(token_pool, entry->lock_token)); + + /* We look for unversioned items in the parent path of any item + that gets added to commitables. This ensures that we find + unversioned items even when individual targets are listed + rather than their parent directories. */ + SVN_ERR(find_unversioned_items(unversioned_items, NULL, p_path, + adm_access, copy_mode, + directory_cache, ctx, pool)); } } @@ -672,6 +862,8 @@ depth_below_here, just_locked, changelists, + unversioned_items, + directory_cache, ctx, loop_pool)); } @@ -733,6 +925,7 @@ svn_depth_t depth, svn_boolean_t just_locked, const apr_array_header_t *changelists, + apr_hash_t *unversioned_items, svn_client_ctx_t *ctx, apr_pool_t *pool) { @@ -740,6 +933,7 @@ svn_wc_adm_access_t *dir_access; apr_pool_t *subpool = svn_pool_create(pool); apr_hash_t *changelist_hash = NULL; + apr_hash_t *directory_cache = NULL; /* It's possible that one of the named targets has a parent that is * itself scheduled for addition or replacement -- that is, the @@ -868,6 +1062,8 @@ dir_access, entry->url, NULL, entry, NULL, FALSE, FALSE, depth, just_locked, changelist_hash, + unversioned_items, + directory_cache, ctx, subpool)); i++; @@ -917,11 +1113,15 @@ return harvest_committables(btn->committables, NULL, pair->src, dir_access, pair->dst, entry->url, entry, NULL, FALSE, TRUE, svn_depth_infinity, - FALSE, NULL, btn->ctx, pool); + FALSE, NULL, NULL, NULL, btn->ctx, pool); + } +/* TODO: Now that harvest_committables is extended to populate the + unversioned_items list, we should extend this function to use that + facility for WC to URL copies */ svn_error_t * svn_client__get_copy_committables(apr_hash_t **committables, const apr_array_header_t *copy_pairs, @@ -1776,13 +1976,22 @@ svn_client__get_log_msg(const char **log_msg, const char **tmp_file, const apr_array_header_t *commit_items, + const apr_array_header_t *unversioned_items, svn_client_ctx_t *ctx, apr_pool_t *pool) { - if (ctx->log_msg_func3) + if (ctx->log_msg_func4) { /* The client provided a callback function for the current API. Forward the call to it directly. */ + return (*ctx->log_msg_func4)(log_msg, tmp_file, commit_items, + unversioned_items, + ctx->log_msg_baton4, pool); + } + else if (ctx->log_msg_func3) + { + /* The client has provided a pre-1.6 API callback. All we + need to do is ignore the unversioned_items argument */ return (*ctx->log_msg_func3)(log_msg, tmp_file, commit_items, ctx->log_msg_baton3, pool); } Index: subversion/libsvn_client/add.c =================================================================== --- subversion/libsvn_client/add.c (revision 33218) +++ subversion/libsvn_client/add.c (working copy) @@ -727,7 +727,7 @@ } SVN_ERR(svn_client__get_log_msg(&log_msg, &tmp_file, commit_items, - ctx, pool)); + NULL, ctx, pool)); if (! log_msg) return SVN_NO_ERROR; Index: subversion/libsvn_client/commit.c =================================================================== --- subversion/libsvn_client/commit.c (revision 33218) +++ subversion/libsvn_client/commit.c (working copy) @@ -690,7 +690,7 @@ APR_ARRAY_PUSH(commit_items, svn_client_commit_item3_t *) = item; SVN_ERR(svn_client__get_log_msg(&log_msg, &tmp_file, commit_items, - ctx, pool)); + NULL, ctx, pool)); if (! log_msg) return SVN_NO_ERROR; if (tmp_file) @@ -1357,8 +1357,10 @@ apr_array_header_t *dirs_to_lock_recursive; svn_boolean_t lock_base_dir_recursive = FALSE; apr_hash_t *committables, *lock_tokens, *tempfiles = NULL, *digests; + apr_hash_t *unversioned_items = NULL; svn_wc_adm_access_t *base_dir_access; apr_array_header_t *commit_items; + apr_array_header_t *unversioned_items_array = NULL; svn_error_t *cmt_err = SVN_NO_ERROR, *unlock_err = SVN_NO_ERROR; svn_error_t *bump_err = SVN_NO_ERROR, *cleanup_err = SVN_NO_ERROR; svn_boolean_t commit_in_progress = FALSE; @@ -1615,6 +1617,10 @@ pool)); } + /* TODO: Only create the unversioned_items hash, if the client + really wants that information. */ + unversioned_items = apr_hash_make(pool); + /* Crawl the working copy for commit items. */ if ((cmt_err = svn_client__harvest_committables(&committables, &lock_tokens, @@ -1623,6 +1629,7 @@ depth, ! keep_locks, changelists, + unversioned_items, ctx, pool))) goto cleanup; @@ -1637,6 +1644,15 @@ SVN_CLIENT__SINGLE_REPOS_NAME, APR_HASH_KEY_STRING)))) goto cleanup; + + /* Use the same hacked name for unversioned_items as we do for + committables. */ + if (unversioned_items) + unversioned_items_array = apr_hash_get(unversioned_items, + SVN_CLIENT__SINGLE_REPOS_NAME, + APR_HASH_KEY_STRING); + else + unversioned_items_array = NULL; /* If our array of targets contains only locks (and no actual file or prop modifications), then we return here to avoid committing a @@ -1658,7 +1674,7 @@ { const char *tmp_file; cmt_err = svn_client__get_log_msg(&log_msg, &tmp_file, commit_items, - ctx, pool); + unversioned_items_array, ctx, pool); if (cmt_err || (! log_msg)) goto cleanup;