Author: kotkov
Date: Tue Dec 6 20:09:44 2022
New Revision: 1905810
URL: http://svn.apache.org/viewvc?rev=1905810&view=rev
Log:
Switch to an alternative implementation of svn_wc__format_from_context().
The previous implementation seems to have a potential issue in how it finds
the parent wcroot. We were sorting the paths in the depth-first order by using
svn_path_compare_paths(). This function orders children in subdirectories
directly after their parents. We would then check if the path of the previous
entry, closest to the `local_abspath`, is its parent path. If it is, we would
use that entry, or stop searching for a parent otherwise.
The problem is that the previous entry in the sort order may point to the
sibling of the current path. For example, if the sorted entries are
["/A", "/A/B", "/A/Z"], and we are looking for "/A/C", we would check
that "/A/B" is not a parent of that path, and stop before testing "/A" that
actually is a parent of the path.
This change switches to the implementation that doesn't require sorting all
context entries and instead checks the parents of the `local_abspath`, picking
the closest one. It also attempts to have predictable behavior in case there's
no such parent, by returning the defaults instead of additionally scanning
through the other context entries that may be unrelated. The fallback code
currently is not triggered in the test suite, and I assume that this should
not affect the visible behavior.
* subversion/include/private/svn_wc_private.h
(svn_wc__format_from_context): Adjust documentation.
* subversion/libsvn_wc/wc_db_wcroot.c
(): Remove include of "private/svn_sorts_private.h".
(svn_wc__format_from_context): Remove TODO. Switch to a new implementation.
Modified:
subversion/trunk/subversion/include/private/svn_wc_private.h
subversion/trunk/subversion/libsvn_wc/wc_db_wcroot.c
Modified: subversion/trunk/subversion/include/private/svn_wc_private.h
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_wc_private.h?rev=1905810&r1=1905809&r2=1905810&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_wc_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_wc_private.h Tue Dec 6
20:09:44 2022
@@ -2232,8 +2232,8 @@ svn_wc__min_supported_format_version(voi
/**
* Set @a format to the format of the nearest parent working copy root of
- * @a local_abspath in @a wc_ctx, or to the oldest format of any root stored
- * there. If @a wc_ctx is empty, return the library's default format.
+ * @a local_abspath in @a wc_ctx, or to the library's default format if
+ * there are no such roots.
*
* Use @a scratch_pool for temporary allocations.
*
Modified: subversion/trunk/subversion/libsvn_wc/wc_db_wcroot.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db_wcroot.c?rev=1905810&r1=1905809&r2=1905810&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db_wcroot.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db_wcroot.c Tue Dec 6 20:09:44
2022
@@ -31,8 +31,6 @@
#include "svn_pools.h"
#include "svn_version.h"
-#include "private/svn_sorts_private.h"
-
#include "wc.h"
#include "adm_files.h"
#include "wc_db_private.h"
@@ -1032,83 +1030,29 @@ svn_wc__db_drop_root(svn_wc__db_t *db,
}
-/*
- * ### FIXME:
- *
- * There must surely be a better way to find the nearest enclosing wcroot of a
- * path than by copying the hash keys to an array and sorting the array.
- *
- * TODO: Convert the svn_wc__db_t::dir_data hash to a sorted dictionary?.
- */
svn_error_t *
svn_wc__format_from_context(int *format,
svn_wc_context_t *wc_ctx,
const char *local_abspath,
apr_pool_t *scratch_pool)
{
- apr_hash_t *const dir_data = wc_ctx->db->dir_data;
- apr_array_header_t *keys;
- int index;
-
- /* This is what we return if we don't find a concrete format version. */
- SVN_ERR(svn_hash_keys(&keys, dir_data, scratch_pool));
- if (0 == keys->nelts)
+ const char *current_path = local_abspath;
+
+ do
{
- *format = SVN_WC__DEFAULT_VERSION;
- return SVN_NO_ERROR;
- }
+ svn_wc__db_wcroot_t *wcroot;
+
+ wcroot = svn_hash_gets(wc_ctx->db->dir_data, current_path);
+ if (wcroot)
+ {
+ *format = wcroot->format;
+ return SVN_NO_ERROR;
+ }
- svn_sort__array(keys, svn_sort_compare_paths);
- index = svn_sort__bsearch_lower_bound(keys, &local_abspath,
- svn_sort_compare_paths);
-
- /* If the previous key is a parent of the local_abspath, use its format. */
- {
- const char *const here = (index >= keys->nelts ? NULL
- : APR_ARRAY_IDX(keys, index, const char*));
- const char *const prev = (index == 0 ? NULL
- : APR_ARRAY_IDX(keys, index - 1, const char*));
-
- if (here)
- {
- const char *const child = svn_dirent_skip_ancestor(here,
local_abspath);
- if (child && !*child)
- {
- /* Found an exact match in the WC context. */
- svn_wc__db_wcroot_t *wcroot = svn_hash_gets(dir_data, here);
- *format = wcroot->format;
- return SVN_NO_ERROR;
- }
- }
-
- if (prev)
- {
- const char *const child = svn_dirent_skip_ancestor(prev,
local_abspath);
- if (child)
- {
- /* Found the parent path in the WC context. */
- svn_wc__db_wcroot_t *wcroot = svn_hash_gets(dir_data, prev);
- *format = wcroot->format;
- return SVN_NO_ERROR;
- }
- }
- }
-
- /* Find the oldest format recorded in the WC context. */
- {
- int oldest_format = SVN_WC__VERSION;
- apr_hash_index_t *hi;
-
- for (hi = apr_hash_first(scratch_pool, dir_data);
- hi;
- hi = apr_hash_next(hi))
- {
- svn_wc__db_wcroot_t *wcroot = apr_hash_this_val(hi);
- if (wcroot->format < oldest_format)
- oldest_format = wcroot->format;
- }
+ current_path = svn_dirent_dirname(current_path, scratch_pool);
+ }
+ while (!svn_dirent_is_root(current_path, strlen(current_path)));
- *format = oldest_format;
- return SVN_NO_ERROR;
- }
+ *format = SVN_WC__DEFAULT_VERSION;
+ return SVN_NO_ERROR;
}
