Martin Furter wrote:
>
> On Tue, 30 Sep 2003, Julian Foad wrote:
>
>>Martin Furter wrote:
>>
>>>On Mon, 29 Sep 2003, Julian Foad wrote:
>>>
>>>>I am wondering about the relationship between these new configurable
>>>>auto-properties and the existing non-configurable automatic properties
>>>>"svn:executable" and "svn:mimetype". The exising non-configurable
>>>>auto-props seem to be applied only on commit, and not on add. It would
>>>>be silly for the two types of auto-props not to work in the same
>>>>situations. Is there a good reason why they don't?
>>>
>>>If i understood it correctly the current behaviour is the following when
>>>importing:
>>>1. check if the file contains a '\0', if yes set the property
>>> svn:mime-type to application/octet-stream
>>>2. check if the file has the executable bit set, if yes add the
>>> property svn:executable
>>>
>>>These properties are applied in subversion/libsvn_wc/adm_ops.c in
>>>import_file() and subversion/libsvn_client/commit.c in svn_wc_add().
>>
>>(The other way round - in adm_ops.c n svn_wc_add and commit.c in import_file.)
>>
>>>When adding a file svn_wc_add() sets these properties and
>>>svn_client__add_auto_props() overwrites them.
>>>When importing svn_client__add_auto_props() adds the properties and
>>>import_file() adds the dfault if they aren't allready set.
>>>So both commands have the same result.
>>
>>Ah, yes. I missed that. I am glad that I was wrong. I was misled
>>because there is no call to "svn_io_detect_mimetype" in
>>subversion/libsvn_client/add.c. The call is at a lower level, in
>>svn_wc_add.
>>
>>The implementations of "svn_io_detect_mimetype" and
>>"svn_io_is_file_executable" are in libsvn_wc because they ar WC-only
>>operations. It seems to me that your new property-configuration
>>function is very similar: it is also a WC-only operation, and so belongs
>>with its two friends in libsvn_wc. I would make it like this:
>>
>>/* Return a hash of configured AND detected properties together */
>>svn_wc_get_auto_props(...)
>>{
>> /* Look up auto-props in the config file (the first half of
>> your svn_client__add_auto_props). */
>>
>> /* Detect mime-type and executable if they were not configured. */
>>
>> /* Return all of these properties in a hash table. */
>>}
>>
>>and then the two existing callers of
>>"svn_io_is_file_executable"/"svn_io_detect_mime_type" would call the
>>single new function instead of those two old functions:
>>
>>import_file(...)
>>{
>> ...
>> properties = svn_wc_get_auto_props(...);
>> for (hi in properties)
>> SVN_ERR (editor->change_file_prop (file_baton, hi->name, hi->value, pool));
>> ...
>>}
>>
>>svn_wc_add(...)
>>{
>> ...
>> if (! copyfrom_url)
>> {
>> properties = svn_wc_get_auto_props(...);
>> for (hi in properties)
>> SVN_ERR (svn_wc_prop_set (hi->name, hi->value, path, parent_access, pool));
>> }
>> ...
>>}
>>
>>This way, the logic of reading the configured properties and detecting
>>two properties and having the former override the latter is all in one
>>place, and there is only one call to each of editor->change_file_prop
>>and svn_wc_prop_set rather than three of each.
>
>
> Yeah, sounds really great but...
> I don't have the config in svn_wc_add().
> Now i remember why i added it to libsvn_client/add.c ;-)
OK, that's a good point.
> To get it working this way i'd have to add ctx->config to the parameters
> of svn_wc_add().
...
> This change sounds really heavy. Are there other options or shall i just
> do that ?
Here is another option. Perhaps auto-properties (both detected and configured) don't belong in libsvn_wc but only in libsvn_client. svn_wc_add already tries to do too much:
subversion/libsvn_client/copy.c:883: Unfortunately, svn_wc_add() is such a mess that it chokes...
So you should move the detection of MIME type and 'executable' out from svn_wc_add into the appropriate caller(s). It seems that there are only two callers that need this functionality; they are the two where you added a call to svn_client__add_auto_props, in 'add_dir_recursive()' and 'add()'.
That would help to clean up svn_wc_add, and also bring the code to handle both types of automatic properties together in libsvn_client.
Would that work well?
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 30 17:03:40 2003