*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 >>>>>>>> >>>>>>>
