libsvn_fs_util

Checkout Tools
  • last updated 5 hours ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates

Changeset 868167 is being indexed.

Consolidate use of sqlite3_prepare into a helper function which passes

the args that we always use, and uses our normal error system.

* subversion/libsvn_fs_util/sqlite-util.h

(svn_fs__sqlite_prepare): Declare.

* subversion/libsvn_fs_util/sqlite-util.c

(svn_fs__sqlite_prepare): Implement.

(check_format): Use svn_fs__sqlite_prepare.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(index_path_mergeinfo, table_has_any_rows_with_rev,

parse_mergeinfo_from_db, get_mergeinfo_for_path,

get_mergeinfo_for_children):

* subversion/libsvn_fs_util/node-origins-sqlite-index.c

(get_origin, set_origin):

Use svn_fs__sqlite_prepare.

Standardize our use of sqlite3_step (which now only appears in one

place!). Use the Subversion error system as much as possible instead

of passing around sqlite_result values.

* subversion/libsvn_fs_util/sqlite-util.h

(svn_fs__sqlite_step_row): New helper function that runs

sqlite3_step, expecting SQLITE_ROW to be returned.

* subversion/libsvn_fs_util/sqlite-util.c

(svn_fs__sqlite_step_done): Reimplement using step_with_expectation.

(step_with_expectation): Guts of svn_fs__sqlite_step_done, now using

svn_fs__sqlite_step.

(svn_fs__sqlite_step_row): New.

(check_format): Use new helpers.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(parse_mergeinfo_from_db): Use new helpers. Also remove a comment

about fallthrough that has been meaningless since r22184.

(get_mergeinfo_for_path, get_mergeinfo_for_children): Use new

helpers.

* subversion/libsvn_fs_util/node-origins-sqlite-index.c

(get_origin): Use new helpers.

If a call to svn_fs_mergeinfo__update_index is ultimately going to be

a no-op, don't perform any write operations at all.

The upshot of this change is that commits that don't change any

svn:mergeinfo will no longer block (or be blocked by) commands that

read from the merge tracking database.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(table_has_any_rows_with_rev): New helper function.

(clean_tables): New helper function to do the cleanup of failed

transactions, factored out of svn_fs_mergeinfo__update_index.

(svn_fs_mergeinfo__update_index): Factor out clean_tables.

* subversion/libsvn_fs_util/sqlite-util.c

* subversion/libsvn_fs_util/sqlite-util.h

(svn_fs__sqlite_step): New helper for calling sqlite3_step when you

don't know if you expect SQLITE_ROW or SQLITE_DONE.

Followup to r28077, fixing a typo.

* subversion/libsvn_fs_util/sqlite-util.c

(svn_fs__sqlite_open): Add missing *.

Followup to r28073: set the case-sensitivity pragma at connection

time, not database initialization time.

(Note, by the way, that in addition to making the (path LIKE "foo/%")

queries correct, it also allows the sqlite query optimization to

transform them into (path >= "foo/" AND path < "foo0") and use the

index. Correctness and efficiency: two great tastes that taste great

together!)

[ Note from the future: perhaps I shouldn't brag about correctness in

revisions that don't typecheck... see r28081. ]

* subversion/libsvn_fs_util/sqlite-util.c

(schema_create_sql): Don't make LIKEs case-sensitive here...

(svn_fs__sqlite_open): ... do it here instead.

We make at least one LIKE query against paths, to find paths that are

children of a given path. This really really really needs to be

case-sensitive!

* subversion/libsvn_fs_util/sqlite-util.c

(schema_create_sql): Make all LIKEs case-sensitive when the database

is created.

Initialize all svn_merge_range_t fields in representation of "" mergeinfo.

See http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=133009

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(no_mergeinfo): Initialize the inheritable field of this svn_merge_range_t

representing mergeinfo of "" to TRUE.

Suggested by: glasser

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(index_path_mergeinfo) Don't use a "mergeinfo" hash as a

"mergeinfo_for_paths" hash. Just pass NULL for result and use the

"cache" argument to get your answer.

In the spirit of r28059, make sure never to do a SELECT query that

could possibly return info about an uncommitted revision.

index_path_mergeinfo wants to find out about mergeinfo that is *older*

than the revision being committed (new_rev), so it should pass

new_rev-1 to get_mergeinfo_for_path. (This shouldn't be a behavior

change, since any row with revision=new_rev was already deleted in

svn_fs_mergeinfo__update_index.)

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(index_path_mergeinfo): Query mergeinfo that is strictly older than

the mergeinfo we're inserting.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

Add a note at the top warning that all queries on the mergeinfo

tables must actively ensure that they don't get rows corresponding

to uncommitted revisions.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(get_mergeinfo_for_path): Redocument, and fix an internal comment.

Review by: kameshj

When a new-enough version of SQLite is detected at 'configure'-time,

employ its thread-safety runtime verification.

* build/ac-macros/sqlite.m4

(SVN_LIB_SQLITE, SVN_SQLITE_CONFIG): Test whether

sqlite3_threadsafe() is available, and if so, set

$threadsafety_runtime_check_avail to "yes". In the former macro,

record that fact by defining the

SVN_HAVE_SQLITE_THREADSAFE_PREDICATE C preprocessor token.

* subversion/libsvn_fs_util/sqlite-util.c

Include svn_types.h

(init_sqlite): Add new function which verifies that SQLite was

compiled in a thread-safe manner when

SVN_HAVE_SQLITE_THREADSAFE_PREDICATE is defined. This could

potentially also/alternately employ apr_dso_load() to check for

SQLite DLLs which have changed since compile-time.

(svn_fs__sqlite_open): Call init_sqlite(), if the library hasn't

already been initialized.

Reviewed by: glasser

philip

  1. … 1 more file in changeset.
In r27922, we started using the sqlite3_prepare_v2 API. However, this

API was only introduced in SQLite 3.3.9, which seems to be a little

too recent for comfort. This revision removes the use of

sqlite3_prepare_v2, replacing it with sqlite3_prepare. This changes

the semantics of the return value of sqlite3_step, so we now make sure

that whenever sqlite3_step has an unexpected return value, we use

sqlite3_finalize to extract a useful return value.

As a side effect, this plugs a few leaks where sqlite3_finalize calls

were missing (for example, the no-rows branch in

parse_mergeinfo_from_db).

* subversion/libsvn_fs_util/sqlite-util.h

(svn_fs__sqlite_stmt_error): Declare new helper function.

* subversion/libsvn_fs_util/sqlite-util.c

(svn_fs__sqlite_stmt_error): Implement new helper function.

(svn_fs__sqlite_step_done, check_format): Use new helper function

and don't use _v2 API.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(index_path_mergeinfo, parse_mergeinfo_from_db,

get_mergeinfo_for_path, get_mergeinfo_for_path,

get_mergeinfo_for_children):

* subversion/libsvn_fs_util/node-origins-sqlite-index.c

(get_origin, set_origin):

Use new helper function and don't use _v2 API.

Audit the APR pool usage of the recursive function get_mergeinfo_for_path()

(issue #3025).

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

Include apr.h for (at least) apr_ssize_t.

(get_mergeinfo_for_path): Add documentation indicating that a

sub-pool should be used with this function.

(index_path_mergeinfo, get_mergeinfo): Introduce sub-pool for use

with the call to get_mergeinfo_for_path(), dup'ing the returned

mergeinfo hash into POOL.

(mergeinfo_hash_dup): Add new function that returns a deep copy of a

hash of paths -> mergeinfo hashes.

Suggested by: glasser

* subversion/libsvn_fs_util/sqlite-util.h

(svn_fs__sqlite_open): Document POOL usage.

Docstring Fix.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(get_mergeinfo_for_path): Fix the Docstring to document all parameters.

Suggested by: kfogel

Replace all use of apr_pool_clear and apr_pool_destroy in our

codebase with the equivalent svn_pool_* functions (consistency is

helpful).

  1. … 24 more files in changeset.
Make sure to copy a return-value string away from SQLite's memory

space.

* subversion/libsvn_fs_util/node-origins-sqlite-index.c

(get_origin): Copy returned string into a pool passed as a new

argument.

(set_origin): Take a pool argument and pass it through to

get_origin.

(svn_fs__set_node_origins, svn_fs__get_node_origin): Adjust.

Found by: pburba

Turn an overcomplicated macro into a function.

* subversion/libsvn_fs_util/sqlite-util.h

(SVN_FS__SQLITE_STEP_DONE): Remove macro, replacing with ...

(svn_fs__sqlite_step_done): ... function declaration.

* subversion/libsvn_fs_util/sqlite-util.c

(svn_fs__sqlite_step_done): Implement.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(index_path_mergeinfo): Adjust.

* subversion/libsvn_fs_util/node-origins-sqlite-index.c

(set_origin): Adjust.

  1. … 1 more file in changeset.
Make svn_fs__set_node_origins ignore errors due to a readonly

database.

I am committing a somewhat hacky change to fs-test.c to test this; I

will revert this in two revisions.

* subversion/include/private/svn_fs_node_origins.h

(svn_fs__set_node_origins): Document that in the case of a readonly

database, this function may be a no-op.

* subversion/libsvn_fs_util/node-origins-sqlite-index.c

(svn_fs__set_node_origins): If the database operations return a

"readonly" error, ignore it.

* subversion/libsvn_fs_util/sqlite-util.h

(SVN_FS__SQLITE_STEP_DONE): Make sure to finalize the statement

before returning an error; otherwise the call to sqlite3_close will

fail.

* subversion/tests/libsvn_fs/fs-test.c

Add some horribly hacky test code to wipe the node_origins table and

set it readonly a little later. Will be reverted in two revisions.

But both tests do pass.

  1. … 3 more files in changeset.
Standardize error handling in SQLite code, so that the SQLITE_READONLY

error is given a different SVN-level error code from other SQLITE

errors. This will allow caches that are "optional" to ignore errors

due to readonly filesystems while still raising more serious errors.

In order to do this, we add a few new helper macros, and make sure to

always use sqlite3_prepare_v2 instead of sqlite3_prepare; the "v2"

version returns the precise error code from sqlite3_step instead of

just returning SQLITE3_ERROR.

* subversion/include/svn_error_codes.h

(SVN_ERR_FS_SQLITE_READONLY): New error code for an attempt to write

to a read-only SQLite db.

* subversion/libsvn_fs_util/sqlite-util.h

(SVN_FS__SQLITE_ERROR_CODE): New macro to convert a SQLite error

code to a SVN error code; either the new "read-only" one or the

generic SVN_ERR_FS_SQLITE_ERROR.

(SVN_FS__SQLITE_ERR): Update to use SVN_FS__SQLITE_ERROR_CODE.

(SVN_FS__SQLITE_STEP_DONE): New macro, encapsulating a common idiom:

run the given statement for one step, throwing the appropriate

error if the statement isn't done.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(index_path_mergeinfo, parse_mergeinfo_from_db,

get_mergeinfo_for_path, get_mergeinfo_for_children):

* subversion/libsvn_fs_util/node-origins-sqlite-index.c

(get_origin, set_origin):

* subversion/libsvn_fs_util/sqlite-util.c

(svn_fs__sqlite_exec, check_format):

Update to use sqlite3_prepare_v2, SVN_FS__SQLITE_STEP_DONE, and/or

SVN_FS__SQLITE_ERROR_CODE as appropriate.

  1. … 2 more files in changeset.
* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(get_mergeinfo_for_path): Add/expand inline comments.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(get_mergeinfo_for_path): Fix mis-handling of local variable

"parentpath" (an svn_stringbuf_t), which was not properly setting

"parentpath->len".

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(parse_mergeinfo_from_db): Document pool usage.

Merge sqlite-node-origins branch to trunk.

There is now a sqlite table for node_origins in the same database file

as mergeinfo (though perhaps it should be moved for concurrency

reasons) and support code in libsvn_fs_util. FSFS uses it (BDB uses a

BDB table); it is currently only filled at CACHE MISS time, though it

should be straightforward to fill it at commit time as well.

  1. … 8 more files in changeset.
More auditing of APR pool usage for mergeinfo indexing (issue #3025).

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(svn_fs_mergeinfo__update_index): Introduce sub-pool to avoid memory

leakage from this function.

Suggested by: glasser

Audit APR pool usage in conjunction with iteration in mergeinfo

indexing (issue #3025).

Auditing of recursion from get_megeinfo_for_path() still to come.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

Include svn_pools.h.

(index_path_mergeinfo): Note that an iterpool would be overkill.

(append_component_to_paths, get_mergeinfo): Document POOL usage, and

tweak code to reference POOL via hash accessor (for protection

against future refactoring).

(get_mergeinfo_for_children, svn_fs_mergeinfo__get_mergeinfo):

Document pool usage. Introduce sub-pool for temporary/loop allocations.

Suggested by: glasser

Follow-up to r26229, do away with the svn_merge_range_inheritance_t enum.

r26229 implemented non-inheritable mergeinfo revision ranges in

svn_merge_range_t. The introduction of non-inheritable ranges meant

previously straightforward operations on svn_merge_range_t, i.e. those

done by the svn_mergeinfo_* and svn_rangelist_* APIs and their helpers,

became a bit more complicated. An obvious example being

"is range '2-3*' equal to '2-3'?" In most cases yes, but not when

eliding mergeinfo, e.g.:

Mergeinfo on 'branch': '/trunk:2-3*'

Mergeinfo on 'branch/D/G': '/trunk/D/G:2-3' --> No elision should occur!

To adjust the behavior of these APIs to correctly account for

inheritability, r26229 introduced the enum svn_merge_range_inheritance_t:

/* Don't take inheritability into consideration. */

svn_rangelist_ignore_inheritance,

/* Inheritability of both ranges must be the same. */

svn_rangelist_equal_inheritance,

/* Inheritability of both ranges must be @c TRUE. */

svn_rangelist_only_inheritable

*BUT* after looking at this for a long time it is clear that the third

value, svn_rangelist_only_inheritable, is not necessary and the APIs need

only a boolean arg to tell them whether to consider inheritability or not

(and in some cases doesn't need *any* argument).

So this change removes svn_merge_range_inheritance_t. It also tweaks

the C tests of the svn_mergeinfo_* and svn_rangelist_* APIs to provide

greater coverage in general and of non-inheritable ranges in particular.

* subversion/include/private/svn_mergeinfo_private.h

(svn_mergeinfo__equals): Change consider_inheritance argument from a

svn_merge_range_inheritance_t to a svn_boolean_t. Improve doc string.

* subversion/include/svn_mergeinfo.h

(svn_mergeinfo_parse): Note "new" restictions from r27851 in doc string.

(svn_mergeinfo_diff, svn_rangelist_diff): Change consider_inheritance

argument from a svn_merge_range_inheritance_t to a svn_boolean_t.

Improve doc string.

(svn_mergeinfo_merge, svn_rangelist_merge): Remove

svn_merge_range_inheritance_t consider_inheritance argument altogether

and update doc string.

* subversion/include/svn_types.h

(svn_merge_range_inheritance_t): Removed.

* subversion/libsvn_client/copy.c

(calculate_target_mergeinfo, wc_to_repos_copy): Update calls to

svn_mergeinfo_merge().

* subversion/libsvn_client/diff.c

(display_mergeinfo_diff): Update call to svn_mergeinfo_diff(), always

considering inheritance.

* subversion/libsvn_client/merge.c

(filter_reflected_revisions): Update call to svn_mergeinfo_diff(),

ignoring inheritance.

(update_wc_mergeinfo): Update calls to svn_rangelist_remove() and

svn_rangelist_merge().

* subversion/libsvn_client/mergeinfo.c

(elide_mergeinfo): Update calls to svn_mergeinfo__equals(), always

considering inheritance.

(svn_client_mergeinfo_get_available): Update call to svn_rangelist_remove,

don't consider inheritance.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(get_mergeinfo_for_children): Update call to svn_mergeinfo_merge().

* subversion/libsvn_repos/log.c

(svn_repos__is_branching_copy): Update call to svn_mergeinfo_diff, don't

consider inheritance.

(get_combined_mergeinfo): Update call to svn_mergeinfo_merge().

(combine_mergeinfo_rangelists): Update call to svn_rangelist_merge().

(get_merged_rev_mergeinfo): Update calls to svn_mergeinfo_diff(), ignoring

inheritance, and svn_mergeinfo_merge().

* subversion/libsvn_repos/rev_hunt.c

(get_merged_path_revisions): Update call to svn_mergeinfo_diff, don't

consider inheritance, and call to svn_mergeinfo_merge().

* subversion/libsvn_subr/mergeinfo.c

(combine_ranges): Change consider_inheritance argument from a

svn_merge_range_inheritance_t to a svn_boolean_t. Tweak the doc

string.

(combine_with_lastrange): Get smarter about how we combine two ranges

with differing inheritability, document said smartness in the doc string.

(range_to_stringbuf): Move definition prior to first call which is now

in merge_with_lastrange().

(combine_with_adjacent_lastrange): New, dedicated helper for

svn_mergeinfo_parse() via parse_revlist().

(parse_revlist): Replace calls to combine_with_lastrange with

combine_with_adjacent_lastrange(). Enforce the svn_mergeinfo_parse

restriction on reversed ranges.

(svn_rangelist_merge, svn_mergeinfo_merge): Remove consider_inheritance

argument and update any callers in this file. Update calls to

combine_with_lastrange to not consider inheritance (and thereby merge

intersecting ranges with different inheritability).

(inheritance_equal): Removed definition.

(range_intersect, range_contains): Replace call to inheritance equal with

simple conditional since the consider_inheritance arg to these functions

was *always* just a boolean so the special handling of

svn_merge_range_inheritance_t argument has never actually done

anything!

(svn_rangelist_intersect, svn_rangelist_remove, svn_rangelist_diff,

walk_mergeinfo_hash_for_diff, svn_mergeinfo_diff, svn_mergeinfo__equals):

Change consider_inheritance argument from a svn_merge_range_inheritance_t

to a svn_boolean_t.

* subversion/libsvn_wc/props.c

(diff_mergeinfo_props): Update call to svn_mergeinfo_diff(), ignoring

inheritance.

(diff_mergeinfo_props, combine_forked_mergeinfo_props): Update calls to

svn_mergeinfo_merge().

* subversion/tests/libsvn_subr/mergeinfo-test.c

(broken_mergeinfo_vals): Expand the coverage of our svn_mergeinfo_parse()

tests to cover not only invalid grammar but also the overlapping, unordered,

and reversed range restrictions of svn_mergeinfo_parse(). Throw some

non-inheritable ranges in for good measure too.

(mergeinfo2, mergeinfo3, mergeinfo4, mergeinfo5, mergeinfo6,

mergeinfo7): Remove, all the tests previously using now have their own

test data.

(test_parse_multi_line_mergeinfo): Remove, this test wasn't very thorough

and we now have good coverage of multi-line parses in

test_merge_mergeinfo().

(test_diff_mergeinfo): Update call to svn_mergeinfo_diff().

(test_merge_mergeinfo): Tweak the test to enable easier testing of many

test cases and add a whole slew of new cases.

(test_rangelist_merge, test_rangelist_diff): New.

(test_funcs): Remove test_parse_multi_line_mergeinfo and add

test_rangelist_merge and test_rangelist_diff.

  1. … 12 more files in changeset.
API cleanup: We don't need to pass a double pointer to the target

mergeinfo hash when using svn_mergeinfo_merge() because that function

doesn't allocate a new hash. It merely updates items inside the hash,

which can't change the hash header's location.

* subversion/include/svn_mergeinfo.h,

* subversion/libsvn_subr/mergeinfo.c

(svn_mergeinfo_merge): Remove an unnecessary level of indirection

from the 'mergeinfo' in/out parameter.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(get_mergeinfo_for_children): Remove an unnecessary level of

indirection from 'path_mergeinfo' in/out parameter. Update call

to svn_mergeinfo_merge().

(svn_fs_mergeinfo__get_mergeinfo_for_tree): Update calls to

get_mergeinfo_for_children().

* subversion/libsvn_repos/log.c

(get_combined_mergeinfo, get_merged_rev_mergeinfo): Update calls to

svn_mergeinfo_merge().

* subversion/libsvn_repos/rev_hunt.c

(get_merged_path_revisions): Update calls to svn_mergeinfo_merge().

* subversion/libsvn_client/copy.c

(calculate_target_mergeinfo, extend_wc_mergeinfo, wc_to_repos_copy):

Update calls to svn_mergeinfo_merge().

* subversion/libsvn_client/merge.c

(mark_mergeinfo_as_inheritable_for_a_range): Update calls to

svn_mergeinfo_merge().

* subversion/libsvn_wc/props.c

(combine_mergeinfo_props, combine_forked_mergeinfo_props): Update

calls to svn_mergeinfo_merge().

* subversion/tests/libsvn_subr/mergeinfo-test.c

(test_merge_mergeinfo): Update calls to svn_mergeinfo_merge().

  1. … 8 more files in changeset.
A follow-up to r27615, clarifying code flow. No functional change,

but likely more robust in terms of subsequent code refactoring.

* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c

(svn_fs_mergeinfo__get_mergeinfo, svn_fs_mergeinfo__get_mergeinfo_for_tree):

Initialize local variable "rev" after checking that ROOT is a

revision root (as opposed to a transaction root, which is an error

condition).