[PATCH] Issue #2105 - Add '--no-ignore' option to 'svn add' and 'svn import' (v5)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Issue #2105 - Add '--no-ignore' option to 'svn add' and 'svn import' (v5)

S.Ramaswamy
Log:

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().

* 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.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

2105_v5.patch (23K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Issue #2105 - Add '--no-ignore' option to 'svn add' and 'svn import' (v5)

Julian Foad
Oh, this is awkward.  I see now that we already have parameters named
"no_ignore" in public APIs, and we can't just make them go away.  Does this
mean that we should be hesitant about calling other parameters "do_ignore",
which is very similar in name but has the opposite sense?  I don't know.
Somebody help us to decide, please!


Apart from that, there are just cosmetic issues.


S.Ramaswamy wrote:

> Log:
>
> 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'

'do_ignore' (here and elsewhere within the log message).

>     (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().
>
> * 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 14905)
> +++ subversion/include/svn_client.h (working copy)
> @@ -630,11 +630,30 @@
>   * @a ctx->notify_func2 with @a ctx->notify_baton2 and the path of the
>   * added item.
>   *
> + * If @a do_ignore is TRUE, 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.3.
>   */
>  svn_error_t *
> +svn_client_add3 (const char *path,
> +                 svn_boolean_t recursive,
> +                 svn_boolean_t force,
> +                 svn_boolean_t do_ignore,
> +                 svn_client_ctx_t *ctx,
> +                 apr_pool_t *pool);
> +
> +/**
> + * Similar to svn_client_add3(), but with the @a do_ignore parameter
> + * always set to @c TRUE.
> + *
> + * @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,
> @@ -744,13 +763,32 @@
>   * Use @a nonrecursive to indicate that imported directories should not
>   * recurse into any subdirectories they may have.
>   *
> + * If @a do_ignore is TRUE, 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 do_ignore,
> +                                 svn_client_ctx_t *ctx,
> +                                 apr_pool_t *pool);
> +
> +/**
> + * Similar to svn_client_import2() except for @a do_ignore set to TRUE
> + * always.

Well, that's getting closer to the wording of the one above.  Please use
exactly the same wording, as there is no reason for it to be different.

> + *
> + * @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,
> @@ -758,7 +796,6 @@
>                                  svn_client_ctx_t *ctx,
>                                  apr_pool_t *pool);
>  
> -

(I was hoping that you'd get rid of these space changes, but I won't insist on
it.  If I were the one to check in this patch, I would revert these space
changes before doing so.  Obviously it is in most respects a trivial matter,
but there is an underlying reason for leaving it alone: if committers often
change the spacing for no particularly good reason, then it would quite likely
be changed back to two spaces by the next committer, back to one space again by
a later committer, and so on, causing unnecessary noise in the project history.)

>  /** @since New in 1.2.
>   *
>   * Commit files or directories into repository, authenticating with
[...]
> Index: subversion/tests/clients/cmdline/import_tests.py
> ===================================================================
[...]
> +  if not match:
> +    ### we should raise a less generic error here. which?

I don't know which less generic error we should raise, and I'm not sure that we
should do so.  Karl said he would just use "svntest.Failure", and you did that,
and I am happy with that.  The comment has now been discussed and Karl
effectively made a decision (that we should not raise a less generic error), so
please now delete the comment.

I'm sorry if it's difficult to understand which comments you should be deleting
and which you should be leaving alone.

> +    raise svntest.Failure
[...]

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Issue #2105 - Add '--no-ignore' option to 'svn add' and 'svn import' (v5)

kfogel
Julian Foad <[hidden email]> writes:
> Oh, this is awkward.  I see now that we already have parameters named
> "no_ignore" in public APIs, and we can't just make them go away.  Does
> this mean that we should be hesitant about calling other parameters
> "do_ignore", which is very similar in name but has the opposite sense?
> I don't know. Somebody help us to decide, please!

Ooops!

Although I agreed with you earlier that a positive-sense parameter was
better, in this case I think consistency should come first.  Let's
stick with 'no_ignore' after all.  Sorry for the confusion Ramaswamy!

Internally, we should do whatever sense reversals make the code come
out clearest, of course.

> I don't know which less generic error we should raise, and I'm not
> sure that we should do so.  Karl said he would just use
> "svntest.Failure", and you did that, and I am happy with that.  The
> comment has now been discussed and Karl effectively made a decision
> (that we should not raise a less generic error), so please now delete
> the comment.

Note that 'raise svntest.Failure' can take a string argument
describing the nature of the failure.  See other tests for examples of
this.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]