You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2005/05/10 10:23:54 UTC
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
value
S.Ramaswamy wrote:
>Changelog:
>
>Make 'svn add' honor svn:ignore property
>
>* subversion/libsvn_client/add.c
> (add_dir_recursive) : Add list of local ignores to default ignores
> list.
>
>
If you're fixing an issue, you should say so in the log message.
>Index: subversion/libsvn_client/add.c
>===================================================================
>--- subversion/libsvn_client/add.c (revision 14667)
>+++ subversion/libsvn_client/add.c (working copy)
>@@ -276,6 +276,7 @@
> apr_int32_t flags = APR_FINFO_TYPE | APR_FINFO_NAME;
> svn_wc_adm_access_t *dir_access;
> apr_array_header_t *ignores;
>+ const svn_string_t *value;
>
>
"value"? That's not a very descriptive name. Something like
"local_ignores" or "dir_ignores" would be more appropriate.
> /* Check cancellation; note that this catches recursive calls too. */
> if (ctx->cancel_func)
>@@ -294,6 +295,12 @@
> SVN_ERR (svn_wc_adm_retrieve (&dir_access, adm_access, dirname, pool));
>
> SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
>+ /* Add local svn:ignore globs to default ignores */
>+ SVN_ERR (svn_wc_prop_get (&value, SVN_PROP_IGNORE,
>+ svn_wc_adm_access_path (adm_access), adm_access,
>+ pool));
>+ if (value != NULL)
>+ svn_cstring_split_append (ignores, value->data, "\n\r", FALSE, pool);
>
> /* Create a subpool for iterative memory control. */
> subpool = svn_pool_create (pool);
>
>
It would be better to add a new function, svn_wc_get_ignores, that
returns the combined default and per-directory ignores list. The fact
that you copied this bit of code from
libsvn_wc/status.c:collec_ignore_patterns isn't exactly a good thing,
either -- we try to avoid code duplication where possible.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Michael W Thelen <mi...@pietdepsi.com>.
Julian Foad wrote:
>> Is any developer able to review this latest version of the patch? If
>> not, I'll file an issue for it in the next day or two.
>
> It's issue #2243 and this version of the patch is already filed there
> ... but thanks for keeping an eye on it and noticing that it hasn't been
> replied to.
Oh! Thanks, for some reason the "issue #2243" in the subject line
didn't make an impression on me, apparently. :-)
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Julian Foad <ju...@btopenworld.com>.
Michael W Thelen wrote:
> S.Ramaswamy wrote:
>>"svn_wc_get_ignores" now uses collect_ignore_patterns. v3 of the patch:
>
> Is any developer able to review this latest version of the patch? If
> not, I'll file an issue for it in the next day or two.
It's issue #2243 and this version of the patch is already filed there ... but
thanks for keeping an eye on it and noticing that it hasn't been replied to.
I'll take a look at it.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Michael W Thelen <mi...@pietdepsi.com>.
S.Ramaswamy wrote:
> "svn_wc_get_ignores" now uses collect_ignore_patterns. v3 of the patch:
Is any developer able to review this latest version of the patch? If
not, I'll file an issue for it in the next day or two.
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
> Thanks. Committed in r15001, with the minor tweak below.
Philip Martin pointed out that it's wrong: it honours the grandparent's
svn:ignore property. Really fixed in r15004, I hope.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Julian Foad <ju...@btopenworld.com>.
S.Ramaswamy wrote:
> v4 with the suggested changes.
>
> Log:
>
> Fix issue #2243. Make 'svn add' honor svn:ignore property.
Thanks. Committed in r15001, with the minor tweak below.
- Julian
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 14982)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -2974,6 +2974,18 @@
> apr_hash_t *config,
> apr_pool_t *pool);
>
> +/** Get the list of ignore patterns from the @c svn_config_t's in the
> + * @a config hash and the local ignore patterns from the directory
> + * in @a adm_access. The default and local ignore patterns are stored in
> + * @a *patterns. Allocate @a *patterns and its contents in @pool.
(I tweaked the wording slightly there to use the "active voice" and match the
comment for svn_wc_get_default_ignores, and corrected "@pool" to "@a pool".)
> + *
> + * @since New in 1.3.
> + */
> +svn_error_t *svn_wc_get_ignores (apr_array_header_t **patterns,
> + apr_hash_t *config,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool);
> +
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by "S.Ramaswamy" <ra...@collab.net>.
v4 with the suggested changes.
Log:
Fix issue #2243. Make 'svn add' honor svn:ignore property.
* subversion/include/svn_wc.h
(svn_wc_get_ignores): New function.
* subversion/libsvn_wc/status.c
(svn_wc_get_ignores): New function.
(collect_ignore_patterns): Allocate patterns in pool.
(get_dir_status): Stop allocating patterns.
* subversion/libsvn_client/add.c
(add_dir_recursive): Use the new function svn_wc_get_ignores()
to get the default and local ignore patterns.
* subversion/libsvn_client/clients/cmdline/basic_tests.py
(basic_add_local_ignores): New test.
(test_list): Run it.
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Julian Foad <ju...@btopenworld.com>.
S.Ramaswamy wrote:
> "svn_wc_get_ignores" now uses collect_ignore_patterns.
That's good.
>
> Changelog:
>
> Fix issue #2243. Make 'svn add' honor svn:ignore property.
>
> * subversion/include/svn_wc.h
> (svn_wc_get_ignores) : Added function prototype and doc string for
> new function 'svn_wc_get_ignores'.
>
> * subversion/libsvn_wc/status.c
> (svn_wc_get_ignores) : New function to return the default and local
> svn:ignore patterns.
* subversion/include/svn_wc.h
* subversion/libsvn_wc/status.c
(svn_wc_get_ignores): New function.
>
> * subversion/libsvn_client/add.c
> (add_dir_recursive) : Use the new function 'svn_wc_get_ignores' to
> get the default and local svn:ignore patterns.
>
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 14700)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -2930,6 +2930,18 @@
> apr_hash_t *config,
> apr_pool_t *pool);
>
> +/** Get the list of ignore patterns from the @c svn_config_t's in the
> + * @a config hash and the local ignore patterns from the directory
> + * in @a adm_access. The default and local ignore patterns are stored in
> + * @a *patterns. Allocate @a *patterns and its contents in @pool.
> + *
> + * @since New in 1.3.
> + */
> +svn_error_t *svn_wc_get_ignores (apr_array_header_t **patterns,
> + apr_hash_t *config,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool);
> +
>
> /** Add @a lock to the working copy for @a path. @a adm_access must contain
> * a write lock for @a path. If @a path is read-only, due to locking
> Index: subversion/libsvn_wc/status.c
> ===================================================================
> --- subversion/libsvn_wc/status.c (revision 14700)
> +++ subversion/libsvn_wc/status.c (working copy)
> @@ -1977,3 +1977,18 @@
> /* Return the new hotness. */
> return new_stat;
> }
> +
> +svn_error_t *
> +svn_wc_get_ignores (apr_array_header_t **patterns,
> + apr_hash_t *config,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool)
> +{
> + apr_array_header_t *default_ignores;
> +
> + SVN_ERR (svn_wc_get_default_ignores (&default_ignores, config, pool));
> + SVN_ERR (collect_ignore_patterns (*patterns, default_ignores, adm_access,
> + pool));
According to your doc string, this is supposed to allocate *patterns, but it
doesn't.
collect_ignore_patterns has a poor doc string that indicates that it allocates
its array, but it doesn't. It would be best to fix collect_ignore_patterns so
that it does allocate its array, which will simplify its existing point of call ...
> +
> + return SVN_NO_ERROR;
> +}
> Index: subversion/libsvn_client/add.c
> ===================================================================
> --- subversion/libsvn_client/add.c (revision 14700)
> +++ subversion/libsvn_client/add.c (working copy)
> @@ -293,7 +293,8 @@
>
> SVN_ERR (svn_wc_adm_retrieve (&dir_access, adm_access, dirname, pool));
>
> - SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
> + ignores = apr_array_make (pool, 1, sizeof (const char *));
... and will also simplify this new point of call.
> + SVN_ERR (svn_wc_get_ignores (&ignores, ctx->config, adm_access, pool));
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off
svn:ignore
Posted by "S.Ramaswamy" <ra...@collab.net>.
"svn_wc_get_ignores" now uses collect_ignore_patterns. v3 of the patch:
Changelog:
Fix issue #2243. Make 'svn add' honor svn:ignore property.
* subversion/include/svn_wc.h
(svn_wc_get_ignores) : Added function prototype and doc string for
new function 'svn_wc_get_ignores'.
* subversion/libsvn_wc/status.c
(svn_wc_get_ignores) : New function to return the default and local
svn:ignore patterns.
* subversion/libsvn_client/add.c
(add_dir_recursive) : Use the new function 'svn_wc_get_ignores' to
get the default and local svn:ignore patterns.
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Branko Čibej <br...@xbc.nu>.
S.Ramaswamy wrote:
>>A quick review:
>>
>>S.Ramaswamy wrote:
>>
>>
>>>Thanks for the review. Not sure where the new function should go -
>>>libsvn_wc/status.c or elsewhere - I put it in status.c, since it has
>>>similar functions. Revised patch:
>>>
>>>
>>Seems a bit unlikely to me that it ought to be a new, fully public API.
>> I'd have thought it should be local to the command-line client, but I
>>haven't thought hard about it.
>>
>>
>>
>>>Fix issue #2243. Make 'svn add' honor svn:ignore property.
>>>
>>>* subversion/include/svn_wc.h
>>> (svn_wc_get_ignores) : Added function prototype and doc string for
>>> new function 'svn_wc_get_ignores'.
>>>
>>>* subversion/libsvn_wc/status.c
>>> (svn_wc_get_ignores) : New function to return the default and
>>> local
>>> svn:ignore patterns.
>>>
>>>
>>Didn't you mostly copy this from somewhere? If so, you should call it
>>from there, rather than just duplicating it.
>>
>>
>
>I think you are talking about svn_wc_get_default_ignores, a public method
>for getting the ignore patterns from the ~/.subversion/config file.
>
No, I think he means the static function collect_ignore_patterns in
libsvn_wc/status.c. I mentioned that in an earlier post of mine, and I
said that we frown on code duplication.
collect_ignore_patterns is called exactly once in status.c, so that call
could be replaced with a call to svn_wc_get_ignores. On the other hand,
it might be better if svn_wc_get_ignores caleld collect_ignore_patterns.
I can't say offhand which option is more elegant, but you should
certainly do one or the other instead of copy-pasting the way you're
doing now.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Julian Foad <ju...@btopenworld.com>.
S.Ramaswamy wrote:
>>Didn't you mostly copy this from somewhere? If so, you should call it
>>from there, rather than just duplicating it.
>
> I think you are talking about svn_wc_get_default_ignores, a public method
I thought that there must already have been some instance of that code, but I
see now that "import" doesn't need to deal with the "svn:ignore" property and
"status" fetches the two sources of ignores completely separately, so there's
no duplication. Sorry for the noise.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by "S.Ramaswamy" <ra...@collab.net>.
> A quick review:
>
> S.Ramaswamy wrote:
>> Thanks for the review. Not sure where the new function should go -
>> libsvn_wc/status.c or elsewhere - I put it in status.c, since it has
>> similar functions. Revised patch:
>
> Seems a bit unlikely to me that it ought to be a new, fully public API.
> I'd have thought it should be local to the command-line client, but I
> haven't thought hard about it.
>
>> Fix issue #2243. Make 'svn add' honor svn:ignore property.
>>
>> * subversion/include/svn_wc.h
>> (svn_wc_get_ignores) : Added function prototype and doc string for
>> new function 'svn_wc_get_ignores'.
>>
>> * subversion/libsvn_wc/status.c
>> (svn_wc_get_ignores) : New function to return the default and
>> local
>> svn:ignore patterns.
>
> Didn't you mostly copy this from somewhere? If so, you should call it
> from there, rather than just duplicating it.
I think you are talking about svn_wc_get_default_ignores, a public method
for getting the ignore patterns from the ~/.subversion/config file. "svn
add" is using this function and not taking into account local per-directory
svn:ignore patterns. There was a thread earlier about a new function for
returning the default (config file) and local ignores - Hence the new
method. The new function is more like a convenience routine that calls
svn_wc_get_default_ignores and two lines from libsvn_wc/status.c to get the
list of all svn:ignore patterns.
Thanks
Ramaswamy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Branko Čibej <br...@xbc.nu>.
Julian Foad wrote:
> A quick review:
>
> S.Ramaswamy wrote:
>
>> Thanks for the review. Not sure where the new function should go -
>> libsvn_wc/status.c or elsewhere - I put it in status.c, since it has
>> similar
>> functions. Revised patch:
>
>
> Seems a bit unlikely to me that it ought to be a new, fully public
> API. I'd have thought it should be local to the command-line client,
> but I haven't thought hard about it.
On the contrary, I think it makes perfect sense to make this a public
API. We have svn_wc_get_default_ignores, and a svn_wc_get_ignores that
returns a directory-specific list is a perfect match.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Julian Foad <ju...@btopenworld.com>.
A quick review:
S.Ramaswamy wrote:
> Thanks for the review. Not sure where the new function should go -
> libsvn_wc/status.c or elsewhere - I put it in status.c, since it has similar
> functions. Revised patch:
Seems a bit unlikely to me that it ought to be a new, fully public API. I'd
have thought it should be local to the command-line client, but I haven't
thought hard about it.
> Fix issue #2243. Make 'svn add' honor svn:ignore property.
>
> * subversion/include/svn_wc.h
> (svn_wc_get_ignores) : Added function prototype and doc string for
> new function 'svn_wc_get_ignores'.
>
> * subversion/libsvn_wc/status.c
> (svn_wc_get_ignores) : New function to return the default and local
> svn:ignore patterns.
Didn't you mostly copy this from somewhere? If so, you should call it from
there, rather than just duplicating it.
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 14685)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -2930,6 +2930,15 @@
> apr_hash_t *config,
> apr_pool_t *pool);
>
> +/** Get the list of ignore patterns from the @c svn_config_t's in the
> + * @a config hash and the local ignore patterns from the directory
> + * in @adm_access. Allocate @a ignores and its contents in @pool.
No arg is named "ignores".
This needs "@since" if this is to be a fully public function like this.
> + */
> +svn_error_t *svn_wc_get_ignores (apr_array_header_t **patterns,
> + apr_hash_t *config,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool);
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by Branko Čibej <br...@xbc.nu>.
S.Ramaswamy wrote:
>+/** Get the list of ignore patterns from the @c svn_config_t's in the
>+ * @a config hash and the local ignore patterns from the directory
>+ * in @adm_access. Allocate @a ignores and its contents in @pool.
>+ */
>+svn_error_t *svn_wc_get_ignores (apr_array_header_t **patterns,
>+ apr_hash_t *config,
>+ svn_wc_adm_access_t *adm_access,
>+ apr_pool_t *pool);
>
>
There's no "ignores" parameter, it's "patterns". A slight misspelling,
should be "@a adm_access", not "@adm_access". And a missing line,
"@since New in 1.3".
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2243 - 'svn add' command not keying off svn:ignore
Posted by "S.Ramaswamy" <ra...@collab.net>.
Thanks for the review. Not sure where the new function should go -
libsvn_wc/status.c or elsewhere - I put it in status.c, since it has similar
functions. Revised patch:
ChangeLog:
Fix issue #2243. Make 'svn add' honor svn:ignore property.
* subversion/include/svn_wc.h
(svn_wc_get_ignores) : Added function prototype and doc string for
new function 'svn_wc_get_ignores'.
* subversion/libsvn_wc/status.c
(svn_wc_get_ignores) : New function to return the default and local
svn:ignore patterns.
* subversion/libsvn_client/add.c
(add_dir_recursive) : Use the new function 'svn_wc_get_ignores' to
get the default and local svn:ignore patterns.