Thanks Mike for reviewing this.

I couldn't apply the patch, I think this line is invalid:
[[[
o+ * of the revision via @a revprop_table.
]]]
(Just noting for future reference)

When building, I got a linking error
[[[
/usr/bin/x86_64-linux-gnu-ld.bfd:
../../../subversion/libsvn_ra_local/.libs/libsvn_ra_local-1.so: undefined
reference to `svn_repos_get_commit_editor6'
]]]

I tried to figure it out and reverted the use of
svn_repos_get_commit_editor6 to svn_repos_get_commit_editor5 - that worked.
The only way I could get it to work with svn_repos_get_commit_editor6 was
to change build.conf :
[[[
Index: build.conf
===================================================================
--- build.conf  (revision 1934529)
+++ build.conf  (working copy)
@@ -1312,7 +1312,7 @@
 path = subversion/tests/libsvn_ra_local
 sources = ra-local-test.c
 install = test
-libs = libsvn_test libsvn_wc libsvn_ra_local libsvn_ra libsvn_fs
libsvn_delta libsvn_subr
+libs = libsvn_repos libsvn_test libsvn_wc libsvn_ra_local libsvn_ra
libsvn_fs libsvn_delta libsvn_subr
        apriconv apr

 #
----------------------------------------------------------------------------
]]]
Don't know if I messed up my WC in any way.

make check run successfully.

make davautocheck and make svnserveautocheck fails with:
[[[
FAIL:  svnsync_authz_tests.py 1: verify that unreadable content is not
synced
W: Unexpected output
W: EXPECTED STDERR (match_all=True):
W: ACTUAL STDERR:
W: | svnsync: E200026: Don't know anything about capability
'commit-allow-rev-props'
W: DIFF STDERR (match_all=True):
W: | --- EXPECTED STDERR (match_all=True)
W: | +++ ACTUAL STDERR
W: | @@ -0,0 +1 @@
W: | +svnsync: E200026: Don't know anything about capability
'commit-allow-rev-props'
W: CWD: /home/dsg/svn_trunk4/subversion/tests/cmdline
W: EXCEPTION: SVNLineUnequal
]]]
(and repeated for all tests)
Should it be resolved by updating test expectations or did I miss something
running the test?

Cheers,
Daniel



Den fre 22 maj 2026 kl 22:19 skrev C. Michael Pilato <[email protected]>:

> Okay, I've touched up the @since and @deprecated lines of the patch to
> point to 1.16 and 1.15, respectively.  I also took a crack at writing a
> formal log message (which helped me also to more thoroughly review the
> change).  I'm re-posting the patch (with inline log) here for something
> like safekeeping.
>
> -- Mike
>
> On Mon, Feb 2, 2026 at 6:51 PM C. Michael Pilato <[email protected]>
> wrote:
>
>> Actually, I did spot something that I think needs correction.  The
>> "@since New in 1.15." annotations should say "New in 1.16", right?
>>
>> -- Mike
>>
>> On Sun, Feb 1, 2026 at 10:19 PM C. Michael Pilato <[email protected]>
>> wrote:
>>
>>> Today, I setup a Subversion development environment for the first time
>>> in ... more years than I can recall.  I can confirm that the test suite
>>> passes with this patch, and that the patch itself makes sense to me.  I
>>> also used it for my own svnsync activity (I do nightly backups of personal
>>> repositories), and it seems to work as advertised.
>>>
>>> Not gonna lie, I feel too "out of the game" to commit this patch
>>> outright.  But, I *am* comfortable giving a +1 on it, for whatever that
>>> nuance might mean.
>>>
>>> -- Mike
>>>
>>> On Sat, Jan 31, 2026 at 2:48 PM Timofei Zhakov <[email protected]>
>>> wrote:
>>>
>>>> On Thu, Jan 29, 2026 at 10:47 PM Jordan Peck via dev <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi all,
>>>>>
>>>>> I'd like to propose a patch that optimises svnsync when synchronizing
>>>>> to
>>>>> local filesystem repositories (file:// URLs). Currently, svnsync
>>>>> performs
>>>>> a two-step process for each revision:
>>>>>
>>>>>   1. Commit the revision content (author/date are set to the current
>>>>>      user and current time)
>>>>>   2. Update svn:author and svn:date revision properties to match the
>>>>>      source repository
>>>>>
>>>>> This works but is inefficient for file:// URLs where we have direct
>>>>> repository access and can set these properties correctly during the
>>>>> initial commit.
>>>>>
>>>>>
>>>>> THE PROBLEM
>>>>> -----------
>>>>>
>>>>> When svnsync replays a revision from a source repository, the commit
>>>>> is created with the local user as the author and the current time as
>>>>> the date. After the commit completes, svnsync then makes separate
>>>>> svn_ra_change_rev_prop2() calls to update svn:author and svn:date to
>>>>> match the source repository.
>>>>>
>>>>> For remote servers (svn://, http://), this two-step process is
>>>>> necessary
>>>>> because the server enforces its own author/date policies. However, for
>>>>> file:// URLs via ra_local, we have direct access to the repository and
>>>>> can bypass this limitation.
>>>>>
>>>>> We have an SVN server acting as the slave via the Apache module proxy,
>>>>> the slave server is kept in sync with the master using svnsync. This
>>>>> two-step approach has caused issues with various tools that monitor the
>>>>> slave SVN server for new revisions, they will occasionally get the data
>>>>> for a synced revision that hasn't yet had the revision properties
>>>>> copied. Having the sync be a single atomic operation means this can
>>>>> never happen.
>>>>>
>>>>>
>>>>> THE SOLUTION
>>>>> ------------
>>>>>
>>>>> This patch introduces a new RA capability "commit-allow-rev-props" that
>>>>> ra_local advertises. When svnsync detects this capability on the
>>>>> destination repository, it:
>>>>>
>>>>>   1. Includes svn:author and svn:date in the initial commit's revprop
>>>>>      table (instead of filtering them out)
>>>>>
>>>>>   2. ra_local now conditionally preserves these properties:
>>>>>      - svn:author is only set to the session user if not already
>>>>> provided
>>>>>      - If svn:date is provided, the SVN_FS_TXN_CLIENT_DATE flag is
>>>>> passed
>>>>>        to svn_fs_begin_txn2() so the filesystem uses that date
>>>>>
>>>>>   3. Skips the post-commit revprop update step since author/date are
>>>>>      already correct
>>>>>
>>>>>
>>>>> CHANGES OVERVIEW
>>>>> ----------------
>>>>>
>>>>> subversion/include/svn_ra.h:
>>>>>   - Added SVN_RA_CAPABILITY_COMMIT_ALLOW_REV_PROPS capability
>>>>> definition
>>>>>
>>>>> subversion/include/svn_repos.h:
>>>>>   - Added svn_repos_fs_begin_txn_for_commit3() with flags parameter
>>>>>   - Added svn_repos_get_commit_editor6() with txn_flags parameter
>>>>>
>>>>> subversion/libsvn_repos/fs-wrap.c:
>>>>>   - Implemented svn_repos_fs_begin_txn_for_commit3() which passes flags
>>>>>     (including SVN_FS_TXN_CLIENT_DATE) to svn_fs_begin_txn2()
>>>>>
>>>>> subversion/libsvn_repos/commit.c:
>>>>>   - Added txn_flags to edit_baton structure
>>>>>   - Implemented svn_repos_get_commit_editor6() to support txn_flags
>>>>>   - Updated open_root() to use the new begin_txn function
>>>>>
>>>>> subversion/libsvn_ra_local/ra_plugin.c:
>>>>>   - Advertises SVN_RA_CAPABILITY_COMMIT_ALLOW_REV_PROPS
>>>>>   - Modified get_commit_editor to preserve caller-provided author/date
>>>>>   - Sets SVN_FS_TXN_CLIENT_DATE when date is provided in revprops
>>>>>
>>>>> subversion/svnsync/svnsync.c:
>>>>>   - Detects the new capability on the destination
>>>>>   - When capability is present, includes author/date in commit revprops
>>>>>   - Skips post-commit revprop sync when capability is present
>>>>>   - Updated user-facing messages to reflect the optimization
>>>>>
>>>>>
>>>>> USER-VISIBLE CHANGES
>>>>> --------------------
>>>>>
>>>>> When synchronizing to a file:// URL, svnsync now displays:
>>>>>
>>>>>   Destination supports commit-time author and date; no post-commit
>>>>>   revprop sync needed.
>>>>>   Committed revision 1.
>>>>>   Committed revision 2.
>>>>>   ...
>>>>>
>>>>> Instead of the previous:
>>>>>
>>>>>   Committed revision 1.
>>>>>   Copied properties for revision 1.
>>>>>   Committed revision 2.
>>>>>   Copied properties for revision 2.
>>>>>   ...
>>>>>
>>>>>
>>>>> BENEFITS
>>>>> --------
>>>>>
>>>>> 1. Atomicity: Author and date are set in the same transaction as the
>>>>>    commit, rather than being updated afterward.
>>>>>
>>>>> 2. No behavioral change for remote servers: The optimization only
>>>>>    applies when the destination advertises the capability, so svn://
>>>>>    and http:// synchronization continues to work as before.
>>>>>
>>>>> 3. Reduced I/O: Eliminates separate revprop change operations for each
>>>>>    revision synced to a file:// destination.
>>>>>
>>>>> COMPATIBILITY
>>>>> -------------
>>>>>
>>>>> - The new capability is only advertised by ra_local, so this is a
>>>>>   client-side optimization with no server protocol changes.
>>>>>
>>>>> - Existing svnsync mirrors continue to work unchanged.
>>>>>
>>>>> - The new API functions (svn_repos_fs_begin_txn_for_commit3 and
>>>>>   svn_repos_get_commit_editor6) maintain backward compatibility
>>>>>   through wrapper functions.
>>>>>
>>>>>
>>>>> TESTING
>>>>> -------
>>>>>
>>>>> I have tested this with svnsync to file:// URLs and verified that:
>>>>> - Revision properties (author, date, log) match the source repository
>>>>> - The new optimization message appears at sync start
>>>>> - No "Copied properties" messages appear
>>>>> - Subsequent incremental syncs work correctly
>>>>>
>>>>>
>>>>> Please review and let me know if you have any questions or suggestions.
>>>>>
>>>>> Thanks,
>>>>> Jordan Peck
>>>>>
>>>>
>>>> Hi,
>>>>
>>>> +1
>>>>
>>>> I didn't look into the patch in detail, but the idea sounds good to me.
>>>>
>>>> --
>>>> Timofei Zhakov
>>>>
>>>

Reply via email to