Ramaswamy S wrote:
> ok, v5 with "no_ignore" restored. (prob. with CN mail acct, sending from gmail)
This is looking close to being ready to commit, so I'm reviewing it more
thoroughly (looking at the resulting code, not just the diff).
Thank you very much for removing the spurious changes to white space.
> Fix issue #2105. Add "--no-ignore" switch to 'svn import' and
> 'svn add'.
>
> * subversion/include/svn_client.h
> (svn_client_import2): New prototype.
> (svn_client_import): Deprecate.
> (svn_client_add3): New prototype.
> (svn_client_add2): Deprecate.
>
> * subversion/libsvn_client/commit.c
> (svn_client_import2): New function. Similar to
> svn_client_import() but takes an additional boolean parameter
> 'no_ignore'
> (svn_client_import): Re-implemented using new function
> svn_client_import2().
> (import_dir, import): Take new 'no_ignore' parameter.
>
> * subversion/libsvn_client/add.c
> (add_dir_recursive): New boolean parameter 'no_ignore'.
> (add): Take new boolean parameter 'no_ignore' to function and
> pass it along to add_dir_recursive()
> (svn_client_add3): New function. Similar to svn_client_add2()
> except for the additional boolean parameter 'no_ignore'.
> (svn_client_add2, svn_client_add): Re-implemented using
> svn_client_add3().
> (svn_client_mkdir): Use new function svn_client_add3().
>
> * subversion/clients/cmdline/import-cmd.c
> (svn_cl__import): Call svn_client_import2().
>
You haven't mentioned the file "add-cmd.c".
> * subversion/clients/cmdline/main.c
> (svn_cl__cmd_table): New '--no-ignore' option for import and add.
>
> * subversion/tests/clients/cmdline/import_tests.py
> (import_no_ignores): New test.
> (test_list): Run it.
>
> * subversion/tests/clients/cmdline/basic_tests.py
> (basic_add_no_ignores): New test.
> (test_list): Run it.
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 14951)
> +++ subversion/include/svn_client.h (working copy)
> @@ -630,13 +630,30 @@
> * @a ctx->notify_func2 with @a ctx->notify_baton2 and the path of the
> * added item.
> *
> + * If @a no_ignore is FALSE, don't add files or directories that match
> + * ignore patterns.
> + *
> * Important: this is a *scheduling* operation. No changes will
> * happen to the repository until a commit occurs. This scheduling
> * can be removed with svn_client_revert().
> *
> - * @since New in 1.1.
> + * @since New in 1.3.
> */
> svn_error_t *
> +svn_client_add3 (const char *path,
> + svn_boolean_t recursive,
> + svn_boolean_t force,
> + svn_boolean_t no_ignore,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> +/**
> + * Similar to svn_client_add3(), but with the @a no_ignore parameter
> + * always set to @c FALSE.
> + *
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> + */
> +svn_error_t *
> svn_client_add2 (const char *path,
> svn_boolean_t recursive,
> svn_boolean_t force,
> @@ -746,13 +763,32 @@
> * Use @a nonrecursive to indicate that imported directories should not
> * recurse into any subdirectories they may have.
> *
> + * If @a no_ignore is FALSE, don't add files or directories that match
> + * ignore patterns.
> + *
> * ### kff todo: This import is similar to cvs import, in that it does
> * not change the source tree into a working copy. However, this
> * behavior confuses most people, and I think eventually svn _should_
> * turn the tree into a working copy, or at least should offer the
> * option. However, doing so is a bit involved, and we don't need it
> - * right now.
> + * right now.
> + *
> + * @since New in 1.3.
> */
> +svn_error_t *svn_client_import2 (svn_client_commit_info_t **commit_info,
> + const char *path,
> + const char *url,
> + svn_boolean_t nonrecursive,
> + svn_boolean_t no_ignore,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> +/**
> + * Similar to svn_client_import2(), but with @a no_ignore set to FALSE
> + * always.
You've still got unnecessarily different wording here (and an extra space)!
You must think I'm mad to keep on complaining about this :-) It's just a tiny
thing, but it's one of those things that takes just a little bit more time for
the reader to understand it, and just a little bit more inconvenience to people
doing a search or search-and-replace, and so on.
I don't mind whether they say "@a no_ignore" or "the @a no_ignore parameter",
and I don't mind whether they say "set to @c FALSE" or "set to FALSE" or just
"false" (though I prefer the last), but put "always" before "set" because
that's how it is in other existing statements, and make them the same as each
other. Thanks!
> + *
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> + */
> svn_error_t *svn_client_import (svn_client_commit_info_t **commit_info,
> const char *path,
> const char *url,
> Index: subversion/libsvn_client/add.c
> ===================================================================
> --- subversion/libsvn_client/add.c (revision 14951)
> +++ subversion/libsvn_client/add.c (working copy)
> @@ -262,10 +262,23 @@
> return SVN_NO_ERROR;
> }
>
> +/* Schedule directory DIRNAME recursively for addition with access baton
> + * ADM_ACCESS.
> + *
> + * If DIRNAME (or any item below directory DIRNAME) is already scheduled for
> + * addition, add will fail and return an error unless FORCE is TRUE.
> + *
> + * Files and directories that match ignore patterns will not be added unless
> + * NO_IGNORE is TRUE.
> + *
> + * A non-null CTX->CANCEL_FUNC will be called with CTX->CANCEL_BATON if the
> + * user cancels the operation.
That's not quite right: it is called even if the user doesn't cancel. Write
"If CTX->CANCEL_FUNC is non-null, call it with CTX->CANCEL_BATON to allow the
user to cancel the operation." or look at how it is written in other existing
places (some of which are better than others).
> + */
> static svn_error_t *
> add_dir_recursive (const char *dirname,
> svn_wc_adm_access_t *adm_access,
> svn_boolean_t force,
> + svn_boolean_t no_ignore,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> @@ -293,7 +306,8 @@
>
> SVN_ERR (svn_wc_adm_retrieve (&dir_access, adm_access, dirname, pool));
>
> - SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
> + if (!no_ignore)
> + SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
>
> /* Create a subpool for iterative memory control. */
> subpool = svn_pool_create (pool);
> @@ -322,7 +336,8 @@
> || (this_entry.name[1] == '.' && this_entry.name[2] == '\0')))
> continue;
>
> - if (svn_cstring_match_glob_list (this_entry.name, ignores))
> + if ((!no_ignore) && svn_cstring_match_glob_list (this_entry.name,
> + ignores))
Indentation.
> continue;
>
> /* Construct the full path of the entry. */
> @@ -332,7 +347,7 @@
> if (this_entry.filetype == APR_DIR)
> {
> SVN_ERR (add_dir_recursive (fullpath, dir_access, force,
> - ctx, subpool));
> + no_ignore, ctx, subpool));
> }
> else if (this_entry.filetype != APR_UNKFILE)
> {
> @@ -384,6 +399,7 @@
> add (const char *path,
> svn_boolean_t recursive,
> svn_boolean_t force,
> + svn_boolean_t no_ignore,
> svn_wc_adm_access_t *adm_access,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> @@ -393,7 +409,7 @@
>
> SVN_ERR (svn_io_check_path (path, &kind, pool));
> if ((kind == svn_node_dir) && recursive)
> - err = add_dir_recursive (path, adm_access, force, ctx, pool);
> + err = add_dir_recursive (path, adm_access, force, no_ignore, ctx, pool);
> else if (kind == svn_node_file)
> err = add_file (path, ctx, adm_access, pool);
> else
> @@ -413,10 +429,12 @@
>
>
> svn_error_t *
> -svn_client_add (const char *path,
> - svn_boolean_t recursive,
> - svn_client_ctx_t *ctx,
> - apr_pool_t *pool)
> +svn_client_add3 (const char *path,
> + svn_boolean_t recursive,
> + svn_boolean_t force,
> + svn_boolean_t no_ignore,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> {
> svn_error_t *err, *err2;
> svn_wc_adm_access_t *adm_access;
> @@ -426,7 +444,7 @@
> TRUE, 0, ctx->cancel_func, ctx->cancel_baton,
> pool));
>
> - err = add (path, recursive, FALSE, adm_access, ctx, pool);
> + err = add (path, recursive, force, no_ignore, adm_access, ctx, pool);
>
> err2 = svn_wc_adm_close (adm_access);
> if (err2)
> @@ -442,32 +460,22 @@
>
>
> svn_error_t *
> -svn_client_add2 (const char *path,
> +svn_client_add2 (const char *path,
> svn_boolean_t recursive,
> svn_boolean_t force,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> - svn_error_t *err, *err2;
> - svn_wc_adm_access_t *adm_access;
> - const char *parent_path = svn_path_dirname (path, pool);
> + return svn_client_add3 (path, recursive, force, FALSE, ctx, pool);
> +}
>
> - SVN_ERR (svn_wc_adm_open3 (&adm_access, NULL, parent_path,
> - TRUE, 0, ctx->cancel_func, ctx->cancel_baton,
> - pool));
> -
> - err = add (path, recursive, force, adm_access, ctx, pool);
> -
> - err2 = svn_wc_adm_close (adm_access);
> - if (err2)
> - {
> - if (err)
> - svn_error_clear (err2);
> - else
> - err = err2;
> - }
> -
> - return err;
> +svn_error_t *
> +svn_client_add (const char *path,
> + svn_boolean_t recursive,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + return svn_client_add3 (path, recursive, FALSE, FALSE, ctx, pool);
> }
>
>
> @@ -633,7 +641,7 @@
> SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
>
> SVN_ERR (svn_io_dir_make (path, APR_OS_DEFAULT, subpool));
> - err = svn_client_add (path, FALSE, ctx, subpool);
> + err = svn_client_add3 (path, FALSE, FALSE, FALSE, ctx, subpool);
>
> /* We just created a new directory, but couldn't add it to
> version control. Don't leave unversioned directoies behind. */
> Index: subversion/libsvn_client/commit.c
> ===================================================================
> --- subversion/libsvn_client/commit.c (revision 14951)
> +++ subversion/libsvn_client/commit.c (working copy)
> @@ -283,12 +283,15 @@
> * Accumulate file paths and their batons in FILES, which must be
> * non-null. (These are used to send postfix textdeltas later).
> *
> - * If CTX->NOTIFY_FUNC is non-null, invoke it with CTX->NOTIFY_BATON for each
> - * directory.
> + * NO_IGNORE is a boolean to determine if directories matching ignore
> + * patterns should be imported.
That's imprecise. Not just directories, but files too, surely? If so, say it
the same way you did above for add_dir_recursive:
"Files and directories that match ignore patterns will not be imported unless
NO_IGNORE is TRUE."
... or like you did for svn_client_add3:
"If NO_IGNORE is FALSE, don't import files or directories that match ignore
patterns."
Those are already two different ways of saying the same thing, and it wouldn't
hurt to make them the same as each other too.
> *
> * EXCLUDES is a hash whose keys are absolute paths to exclude from
> * the import (values are unused).
> *
> + * If CTX->NOTIFY_FUNC is non-null, invoke it with CTX->NOTIFY_BATON for each
> + * directory.
> + *
> * Use POOL for any temporary allocation. */
> static svn_error_t *
> import_dir (const svn_delta_editor_t *editor,
> @@ -297,6 +300,7 @@
> const char *edit_path,
> svn_boolean_t nonrecursive,
> apr_hash_t *excludes,
> + svn_boolean_t no_ignore,
> import_ctx_t *import_ctx,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> @@ -308,7 +312,8 @@
>
> SVN_ERR (svn_path_check_valid (path, pool));
>
> - SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
> + if (!no_ignore)
> + SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
>
> SVN_ERR (svn_io_get_dirents (&dirents, path, pool));
>
> @@ -364,7 +369,7 @@
> if (apr_hash_get (excludes, abs_path, APR_HASH_KEY_STRING))
> continue;
>
> - if (svn_cstring_match_glob_list (filename, ignores))
> + if ((!no_ignore) && svn_cstring_match_glob_list (filename, ignores))
> continue;
>
> /* We only import subdirectories when we're doing a regular
> @@ -399,9 +404,10 @@
> }
>
> /* Recurse. */
> - SVN_ERR (import_dir (editor, this_dir_baton,
> - this_path, this_edit_path,
> - FALSE, excludes, import_ctx, ctx, subpool));
> + SVN_ERR (import_dir (editor, this_dir_baton, this_path,
> + this_edit_path, FALSE, excludes,
> + no_ignore, import_ctx, ctx,
> + subpool));
>
> /* Finally, close the sub-directory. */
> SVN_ERR (editor->close_directory (this_dir_baton, subpool));
> @@ -438,12 +444,15 @@
> * the names of intermediate directories that are created before the
> * real import begins. NEW_ENTRIES may NOT be NULL.
> *
> + * EXCLUDES is a hash whose keys are absolute paths to exclude from
> + * the import (values are unused).
> + *
> + * NO_IGNORE is a boolean to determine if files and directories matching
> + * ignore patterns should be imported.
Same here - say it the same way as in other places.
> + *
> * If CTX->NOTIFY_FUNC is non-null, invoke it with CTX->NOTIFY_BATON for
> * each imported path, passing actions svn_wc_notify_commit_added.
> *
> - * EXCLUDES is a hash whose keys are absolute paths to exclude from
> - * the import (values are unused).
> - *
> * Use POOL for any temporary allocation.
> *
> * Note: the repository directory receiving the import was specified
> @@ -458,6 +467,7 @@
> void *edit_baton,
> svn_boolean_t nonrecursive,
> apr_hash_t *excludes,
> + svn_boolean_t no_ignore,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> @@ -522,15 +532,22 @@
>
> if (kind == svn_node_file)
> {
> - SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
> - if (! svn_cstring_match_glob_list (path, ignores))
> + svn_boolean_t ignores_match = FALSE;
> +
> + if (!no_ignore)
> + {
> + SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
> + ignores_match = svn_cstring_match_glob_list (path, ignores);
> + }
> + if (!ignores_match)
> SVN_ERR (import_file (editor, root_baton, path, edit_path,
> import_ctx, ctx, pool));
> }
> else if (kind == svn_node_dir)
> {
> SVN_ERR (import_dir (editor, root_baton, path, edit_path,
> - nonrecursive, excludes, import_ctx, ctx, pool));
> + nonrecursive, excludes, no_ignore, import_ctx,
> + ctx, pool));
>
> }
> else if (kind == svn_node_none)
> @@ -616,12 +633,13 @@
> /*** Public Interfaces. ***/
>
> svn_error_t *
> -svn_client_import (svn_client_commit_info_t **commit_info,
> - const char *path,
> - const char *url,
> - svn_boolean_t nonrecursive,
> - svn_client_ctx_t *ctx,
> - apr_pool_t *pool)
> +svn_client_import2 (svn_client_commit_info_t **commit_info,
> + const char *path,
> + const char *url,
> + svn_boolean_t nonrecursive,
> + svn_boolean_t no_ignore,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> {
> svn_error_t *err = SVN_NO_ERROR;
> const char *log_msg = "";
> @@ -743,7 +761,7 @@
> /* If an error occurred during the commit, abort the edit and return
> the error. We don't even care if the abort itself fails. */
> if ((err = import (path, new_entries, editor, edit_baton,
> - nonrecursive, excludes, ctx, subpool)))
> + nonrecursive, excludes, no_ignore, ctx, subpool)))
> {
> svn_error_clear (editor->abort_edit (edit_baton, subpool));
> return err;
> @@ -768,6 +786,17 @@
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_client_import (svn_client_commit_info_t **commit_info,
> + const char *path,
> + const char *url,
> + svn_boolean_t nonrecursive,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + return svn_client_import2 (commit_info, path, url, nonrecursive,
> + FALSE, ctx, pool);
> +}
>
> static svn_error_t *
> remove_tmpfiles (apr_hash_t *tempfiles,
> Index: subversion/clients/cmdline/add-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/add-cmd.c (revision 14951)
> +++ subversion/clients/cmdline/add-cmd.c (working copy)
> @@ -63,8 +63,9 @@
>
> svn_pool_clear (subpool);
> SVN_ERR (svn_cl__check_cancel (ctx->cancel_baton));
> - err = svn_client_add2 (target, (! opt_state->nonrecursive),
> - opt_state->force, ctx, subpool);
> + err = svn_client_add3 (target, (! opt_state->nonrecursive),
> + opt_state->force, opt_state->no_ignore,
> + ctx, subpool);
> if (err)
> {
> if (err->apr_err == SVN_ERR_ENTRY_EXISTS)
> Index: subversion/clients/cmdline/import-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/import-cmd.c (revision 14951)
> +++ subversion/clients/cmdline/import-cmd.c (working copy)
> @@ -107,12 +107,13 @@
> SVN_ERR (svn_cl__make_log_msg_baton (&(ctx->log_msg_baton), opt_state,
> NULL, ctx->config, pool));
> SVN_ERR (svn_cl__cleanup_log_msg
> - (ctx->log_msg_baton, svn_client_import (&commit_info,
> - path,
> - url,
> - opt_state->nonrecursive,
> - ctx,
> - pool)));
> + (ctx->log_msg_baton, svn_client_import2 (&commit_info,
> + path,
> + url,
> + opt_state->nonrecursive,
> + opt_state->no_ignore,
> + ctx,
> + pool)));
>
> if (commit_info && ! opt_state->quiet)
> SVN_ERR (svn_cl__print_commit_info (commit_info, pool));
> Index: subversion/clients/cmdline/main.c
> ===================================================================
> --- subversion/clients/cmdline/main.c (revision 14951)
> +++ subversion/clients/cmdline/main.c (working copy)
> @@ -183,7 +183,8 @@
> "them for addition to repository. They will be added in next commit.\n"
> "usage: add PATH...\n"),
> {svn_cl__targets_opt, 'N', 'q', svn_cl__config_dir_opt,
> - svn_cl__force_opt, svn_cl__autoprops_opt, svn_cl__no_autoprops_opt} },
> + svn_cl__force_opt, svn_cl__no_ignore_opt, svn_cl__autoprops_opt,
> + svn_cl__no_autoprops_opt} },
>
> { "blame", svn_cl__blame, {"praise", "annotate", "ann"},
> N_("Output the content of specified files or\n"
> @@ -351,7 +352,8 @@
> " If PATH is a directory, the contents of the directory are added\n"
> " directly under URL.\n"),
> {'q', 'N', svn_cl__autoprops_opt, svn_cl__no_autoprops_opt,
> - SVN_CL__LOG_MSG_OPTIONS, SVN_CL__AUTH_OPTIONS, svn_cl__config_dir_opt} },
> + SVN_CL__LOG_MSG_OPTIONS, svn_cl__no_ignore_opt, SVN_CL__AUTH_OPTIONS,
> + svn_cl__config_dir_opt} },
>
> { "info", svn_cl__info, {0},
> N_("Display information about a local or remote item.\n"
> Index: subversion/tests/clients/cmdline/import_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/import_tests.py (revision 14951)
> +++ subversion/tests/clients/cmdline/import_tests.py (working copy)
> @@ -189,6 +189,82 @@
> None, None, 1)
>
> #----------------------------------------------------------------------
> +def import_no_ignores(sbox):
> + 'import ignored files in imported dirs'
> +
> + # import ignored files using the "--no-ignore" option
> +
> + sbox.build()
> + wc_dir = sbox.wc_dir
> +
> + dir_path = os.path.join(wc_dir, 'dir')
> + foo_c_path = os.path.join(dir_path, 'foo.c')
> + foo_o_path = os.path.join(dir_path, 'foo.o')
> + foo_lo_path = os.path.join(dir_path, 'foo.lo')
> + foo_rej_path = os.path.join(dir_path, 'foo.rej')
> +
> + os.mkdir(dir_path, 0755)
> + open(foo_c_path, 'w')
> + open(foo_o_path, 'w')
> + open(foo_lo_path, 'w')
> + open(foo_rej_path, 'w')
> +
> + # import new dir into repository
> + url = svntest.main.current_repo_url + '/dir'
> +
> + output, errput = svntest.actions.run_and_verify_svn(
> + None, None, [], 'import',
> + '--username', svntest.main.wc_author,
> + '--password', svntest.main.wc_passwd,
> + '-m', 'Log message for new import', '--no-ignore',
> + dir_path, url)
> +
> + lastline = string.strip(output.pop())
> + cm = re.compile ("(Committed|Imported) revision [0-9]+.")
> + match = cm.search (lastline)
> + if not match:
> + raise svntest.Failure
> +
> + # remove (uncontrolled) local dir
> + svntest.main.safe_rmtree(dir_path)
> +
> + # Create expected disk tree for the update (disregarding props)
> + expected_disk = svntest.main.greek_state.copy()
> + expected_disk.add({
> + 'dir/foo.c' : Item(''),
> + 'dir/foo.o' : Item(''),
> + 'dir/foo.lo' : Item(''),
> + 'dir/foo.rej' : Item(''),
> + })
> +
> + # Create expected status tree for the update (disregarding props).
> + # Newly imported file should be at revision 2.
> + expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
> + expected_status.add({
> + 'dir' : Item(status=' ', wc_rev=2),
> + 'dir/foo.c' : Item(status=' ', wc_rev=2),
> + 'dir/foo.o' : Item(status=' ', wc_rev=2),
> + 'dir/foo.lo' : Item(status=' ', wc_rev=2),
> + 'dir/foo.rej' : Item(status=' ', wc_rev=2),
> + })
> +
> + # Create expected output tree for the update.
> + expected_output = svntest.wc.State(wc_dir, {
> + 'dir' : Item(status='A '),
> + 'dir/foo.c' : Item(status='A '),
> + 'dir/foo.o' : Item(status='A '),
> + 'dir/foo.lo' : Item(status='A '),
> + 'dir/foo.rej' : Item(status='A '),
> + })
> +
> + # do update and check three ways
> + svntest.actions.run_and_verify_update(wc_dir,
> + expected_output,
> + expected_disk,
> + expected_status,
> + None, None, None,
> + None, None, 1)
> +#----------------------------------------------------------------------
> def import_avoid_empty_revision(sbox):
> "avoid creating empty revisions with import"
>
> @@ -225,6 +301,7 @@
> Skip(import_executable, (os.name != 'posix')),
> import_ignores,
> import_avoid_empty_revision,
> + import_no_ignores,
> ]
>
> if __name__ == '__main__':
> Index: subversion/tests/clients/cmdline/basic_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/basic_tests.py (revision 14951)
> +++ subversion/tests/clients/cmdline/basic_tests.py (working copy)
> @@ -1440,6 +1440,35 @@
>
>
> #----------------------------------------------------------------------
> +def basic_add_no_ignores(sbox):
> + 'add ignored files in added dirs'
> +
> + # add ignored files using the '--no-ignore' option
> + sbox.build()
> + wc_dir = sbox.wc_dir
> +
> + dir_path = os.path.join(wc_dir, 'dir')
> + foo_c_path = os.path.join(dir_path, 'foo.c')
> + # add a few files that match typical ignore patterns
> + foo_o_path = os.path.join(dir_path, 'foo.o')
> + foo_lo_path = os.path.join(dir_path, 'foo.lo')
> + foo_rej_path = os.path.join(dir_path, 'foo.rej')
> +
> + os.mkdir(dir_path, 0755)
> + open(foo_c_path, 'w')
> + open(foo_o_path, 'w')
> + open(foo_lo_path, 'w')
> + open(foo_rej_path, 'w')
> +
> + output, err = svntest.actions.run_and_verify_svn(
> + "No output where some expected", SVNAnyOutput, None,
> + 'add', '--no-ignore', dir_path)
> +
> + for line in output:
> + # If we don't see ignores in the add output, fail the test.
> + if not re.match(r'^A\s+.*(foo.(o|rej|lo|c)|dir)$', line):
> + raise svntest.actions.SVNUnexpectedOutput
> +#----------------------------------------------------------------------
> def uri_syntax(sbox):
> 'make sure URI syntaxes are parsed correctly'
>
> @@ -1537,6 +1566,7 @@
> uri_syntax,
> basic_checkout_file,
> basic_info,
> + basic_add_no_ignores,
> ### todo: more tests needed:
> ### test "svn rm http://some_url"
> ### not sure this file is the right place, though.
During manual testing I found:
The ignore lists apply only to items found during recursion, not to items named
as direct targets. This is not a problem with your patch, and may be the
desired behaviour, but it is undocumented where ignores are documented in
".subversion/config" and in "svn help propset".
You should update the template for the Subversion user-level "config" file as
part of your patch, as it presently says:
### Set global-ignores to a set of whitespace-delimited globs
### which Subversion will ignore in its `status' output.
which impies that "global-ignores" applies only to the "status" command.
An uncommitted "svn:ignore" property is not used. If you schedule a directory
"d" for addition, then set its "svn:ignore" property, then schedule its
children for addition using "svn add d --force" (where "force" overrides
skipping it with a warning), the new "svn:ignore" property is not used. It
seems that only the checked-in ("base") value is used. However, it _is_ used
for the "status" command. I think this inconsistency is a problem.
Thank you for your patience. I'm sorry that I keep finding more things to
complain about, but as Karl recently explained to somebody else, the closer
your patch comes to being committed, the more valuable and important it is, so
the more effort people like me are prepared to spend on reviewing it.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jun 3 16:09:25 2005