On Mon, Apr 25, 2011 at 1:26 PM, Hyrum K Wright <hy...@hyrumwright.org> wrote:
> On Tue, Apr 12, 2011 at 4:47 AM, <rhuij...@apache.org> wrote:
>> Author: rhuijben
>> Date: Tue Apr 12 09:47:06 2011
>> New Revision: 1091349
>>
>> URL: http://svn.apache.org/viewvc?rev=1091349&view=rev
>> Log:
>> Use one db operation instead of a few when looking whether to set read-only
>> and executable.
>>
>> * subversion/libsvn_wc/translate.c
>> (svn_wc__maybe_set_executable,
>> svn_wc__maybe_set_read_only): Use svn_wc__db_read_node_install_info
>> instead of multiple db operations to determine the state for these
>> attributes.
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_wc/translate.c
>>
>> Modified: subversion/trunk/subversion/libsvn_wc/translate.c
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/translate.c?rev=1091349&r1=1091348&r2=1091349&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_wc/translate.c (original)
>> +++ subversion/trunk/subversion/libsvn_wc/translate.c Tue Apr 12 09:47:06
>> 2011
>> @@ -350,24 +350,35 @@ svn_wc__maybe_set_executable(svn_boolean
>> apr_pool_t *scratch_pool)
>> {
>> #ifndef WIN32
>> - const svn_string_t *propval;
>> + svn_wc__db_status_t status;
>> + svn_wc__db_kind_t kind;
>> + svn_wc__db_lock_t *lock;
>> + apr_hash_t *props;
>> +
>> + if (did_set)
>> + *did_set = FALSE;
>>
>> SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
>>
>> - SVN_ERR(svn_wc__internal_propget(&propval, db, local_abspath,
>> - SVN_PROP_EXECUTABLE, scratch_pool,
>> - scratch_pool));
>> - if (propval != NULL)
>> - {
>> - SVN_ERR(svn_io_set_file_executable(local_abspath, TRUE, FALSE,
>> - scratch_pool));
>> - if (did_set)
>> - *did_set = TRUE;
>> - }
>> - else
>> + SVN_ERR(svn_wc__db_read_node_install_info(NULL, &status, &kind, NULL,
>> NULL,
>> + &props,
>> + db, local_abspath,
>> + scratch_pool, scratch_pool));
>> +
>> + if (kind != svn_wc__db_kind_file
>> + || status != svn_wc__db_status_normal
>> + || props == NULL
>> + || ! apr_hash_get(props, SVN_PROP_EXECUTABLE, APR_HASH_KEY_STRING))
>> + return SVN_NO_ERROR; /* Not executable */
>> +
>> + SVN_ERR(svn_io_set_file_executable(local_abspath, TRUE, FALSE,
>> + scratch_pool));
>> + if (did_set)
>> + *did_set = TRUE;
>> +#else
>> + if (did_set)
>> + *did_set = FALSE;
>> #endif
>> - if (did_set)
>> - *did_set = FALSE;
>>
>> return SVN_NO_ERROR;
>> }
>> @@ -379,41 +390,39 @@ svn_wc__maybe_set_read_only(svn_boolean_
>> const char *local_abspath,
>> apr_pool_t *scratch_pool)
>> {
>> - const svn_string_t *needs_lock;
>> svn_wc__db_status_t status;
>> svn_wc__db_kind_t kind;
>> svn_wc__db_lock_t *lock;
>> + apr_hash_t *props;
>>
>> if (did_set)
>> *did_set = FALSE;
>>
>> SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
>>
>> - SVN_ERR(svn_wc__db_read_info(&status, &kind, NULL, NULL, NULL, NULL,
>> - NULL, NULL, NULL, NULL, NULL,
>> - NULL, NULL, NULL, NULL, NULL, NULL,
>> - NULL, NULL, NULL, NULL, NULL, NULL,
>> - &lock,
>> - db, local_abspath, scratch_pool,
>> scratch_pool));
>> + SVN_ERR(svn_wc__db_read_node_install_info(NULL, &status, &kind, NULL,
>> NULL,
>> + &props,
>> + db, local_abspath,
>> + scratch_pool, scratch_pool));
>
> These functions are run from within work queue items. If the action
> creating the work queue items changes the properties, the above won't
> pick up those changes. svn_wc__db_read_node_install_info() is
> documented to return the pristine properties, when we want/need the
> current properties here.
In the interests of correctness, I've updated the call in both
functions in r1096555.
>> +
>> + if (kind != svn_wc__db_kind_file
>> + || status != svn_wc__db_status_normal
>> + || props == NULL
>> + || ! apr_hash_get(props, SVN_PROP_NEEDS_LOCK, APR_HASH_KEY_STRING))
>> + return SVN_NO_ERROR; /* Doesn't need lock handling */
>> +
>> + SVN_ERR(svn_wc__db_base_get_info(NULL, NULL, NULL, NULL, NULL, NULL, NULL,
>> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
>> + &lock, NULL,
>> + db, local_abspath,
>> + scratch_pool, scratch_pool));
>>
>> if (lock)
>> return SVN_NO_ERROR; /* We have a lock */
>> - else if (kind != svn_wc__db_kind_file)
>> - return SVN_NO_ERROR;
>>
>> - /* Files that aren't in the repository yet should be left writable. */
>> - if (status != svn_wc__db_status_normal)
>> - return SVN_NO_ERROR;
>> -
>> - SVN_ERR(svn_wc__internal_propget(&needs_lock, db, local_abspath,
>> - SVN_PROP_NEEDS_LOCK, scratch_pool,
>> - scratch_pool));
>> - if (needs_lock != NULL)
>> - {
>> - SVN_ERR(svn_io_set_file_read_only(local_abspath, FALSE,
>> scratch_pool));
>> - if (did_set)
>> - *did_set = TRUE;
>> - }
>> + SVN_ERR(svn_io_set_file_read_only(local_abspath, FALSE, scratch_pool));
>> + if (did_set)
>> + *did_set = TRUE;
>>
>> return SVN_NO_ERROR;
>> }
>>
>>
>>
>
