Hi guys, thanks for looking into this in depth. It's been a while since I made the change now but I can have a look into the issues you're discussing. I didn't run any of the tests when making the change so it's not too surprising.
On Mon, 25 May 2026 at 17:09, C. Michael Pilato <[email protected]> wrote: > *ra*test linking:* Interesting... I wonder why it didn't bother my own > build? > > *deprecated.c:* I spotted the same inconsistency, but can't explain it. > (Agree on the need for a follow-up commit for relocating other > deprecated-destined methods.) > > *svnsync error: *Not sure why you're seeing the error. My nightly sync > of repositories does an https-to-file sync, too, and seems to work fine. > Here are the logs from Saturday morning's cron-driven run, which show the > new notification: > > Sat May 23 01:00:01 AM EDT 2026 Syncing repositories >> Syncing cmpilato.repos >> Destination supports commit-time author and date; no post-commit revprop >> sync needed. >> Transmitting file data . >> Committed revision 5206. >> Transmitting file data . >> Committed revision 5207. >> Transmitting file data . >> Committed revision 5208. > > > Ahhhhh, wait a second. You're doing the *opposite* direction -- > file-to-https -- aren't you? Yeah, I didn't test that approach. > > Also, I noticed on a clean build that there remain some other places where > now-deprecated methods are still used. > > subversion/libsvn_repos/fs-wrap.c: In function >> 'svn_repos_fs_begin_txn_for_commit': >> subversion/libsvn_repos/fs-wrap.c:207:3: warning: >> 'svn_repos_fs_begin_txn_for_commit2' is deprecated >> [-Wdeprecated-declarations] >> 207 | return svn_repos_fs_begin_txn_for_commit2(txn_p, repos, rev, >> revprop_table, >> | ^~~~~~ >> subversion/libsvn_repos/fs-wrap.c:181:1: note: declared here >> 181 | svn_repos_fs_begin_txn_for_commit2(svn_fs_txn_t **txn_p, >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > So overall, it seems more work may be needed here. > > Jordan, I don't want to "take over" your contribution -- my patch > revisions aimed to clean up just the kinds of administrivia that get > understandably overlooked. You clearly had the chops to make the initial > patch, so do you have the bandwidth to respond to the concerns in this > thread with a followup? > > -- Mike > > PS: Daniel, my God help you, 'cause if I'm your "only hope", you are, for > all intents and purposes, hopeless. :-) > > On Sun, May 24, 2026 at 4:57 PM Daniel Sahlberg < > [email protected]> wrote: > >> No need to be sorry.. I have enjoyed trying to figure this out, learnt a >> lot about the build system! >> >> Unfortunately I think the patch to build.conf is patching the wrong >> target, it seems to patch ra-test and not ra-local-test. I'm still running >> with the patch of build.conf in my earlier mail. >> >> One thing I still don't understand is why ra-local-test suddenly need the >> lib_repos library... if someone could explain it I would be happy! >> >> One more thing I don't understand: What is the purpose of deprecated.c? I >> thought compatibility wrappers for deprecated functions go there (and in >> that case, should the implementations of svn_repos_fs_begin_txn_for_commit2 >> and svn_repos_get_commit_editor5 be moved there)? However I see we are not >> consistent, when svn_repos_fs_begin_txn_for_commit was deprecated in r >> 863946 <https://svn.apache.org/r863946>, it was kept in fs-wrap.c. If we >> should use deprecated.c, svn_repos_fs_begin_txn should probably move there >> in a separate commit... >> >> I continued to investigate the davautocheck errors and tried to svnsync >> to a repo on my main server (running Debian 12 based Subversion 1.14.2). I >> got the following error message: >> [[[ >> dsg@devi-25-01:~/svn_trunk4/subversion/svnsync$ ./svnsync initialize >> https://svn.example.com/svn/synctest file:///home/dsg/repo/ >> Copied properties for revision 0. >> dsg@devi-25-01:~/svn_trunk4/subversion/svnsync$ ./svnsync sync >> https://svn.example.com/svn/synctest file:///home/dsg/repo/ >> subversion/svnsync/svnsync.c:1670, >> subversion/svnsync/svnsync.c:1599, >> subversion/libsvn_ra_serf/options.c:785: >> (apr_err=SVN_ERR_UNKNOWN_CAPABILITY) >> svnsync: E200026: Don't know anything about capability >> 'commit-allow-rev-props' >> dsg@devi-25-01:~/svn_trunk4/subversion/svnsync$ >> ]]] >> I can read the error message well enough but I have absolutely know idea >> where to start next... I think the quote goes: "Help me C. Michael Pilato, >> you are my only hope" :-) >> >> (Until the error above is resolved, I'm inclined to say -1 to this patch. >> However I'm very much a fan of this idea so I'd love to see it worked out). >> >> Thanks >> Daniel >> >> PS. Full disclosure: './configure --prefix=/home/dsg/bin >> --with-serf=/home/dsg/bin --enable-maintainer-mode --enable-debug >> --enable-svnbrowse' and I'm running on a trunk build of Serf (2.0). >> >> >> Den sön 24 maj 2026 kl 04:23 skrev C. Michael Pilato <[email protected] >> >: >> >>> Okay, this patch includes the build.conf change, plus fixes some >>> deprecation warning caused by newly deprecated methods not getting revved, >>> and it passes "make check" for me. >>> >>> I know I *can* commit it outright, but I've just been away for too long >>> to have that kind of confidence right now. So I'm asking (along with >>> Jordan) for additional eyes on the work. >>> >>> -- Mike >>> >>> PS: I configured this working copy with './configure >>> --enable-maintainer-mode --with-lz4=internal --with-utf8proc=internal >>> --enable-shared', for whatever that's worth. >>> >>> >>> On Sat, May 23, 2026 at 10:05 PM C. Michael Pilato < >>> [email protected]> wrote: >>> >>>> Sorry about this. My update of the patch yesterday wasn't done with a >>>> working copy in hand. I was just literally editing the patch and crafting >>>> the log message (on a machine without a working copy, even). >>>> >>>> Now, as I sit on the original machine I initially reviewed the patch >>>> on, I see I made the same tweak to build.conf as well (back in February >>>> when I first replied). Let me clean this up a bit more and post a version >>>> of the patch that at least works on my machine. :facepalm: >>>> >>>> -- Mike >>>> >>>> On Sat, May 23, 2026 at 8:17 AM Daniel Sahlberg < >>>> [email protected]> wrote: >>>> >>>>> 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 >>>>>>>>> >>>>>>>>
