From 19fd69793708631cc5a04c8956f0ba9ce7e2090f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Mon, 22 Jun 2026 12:59:49 +0100 Subject: [PATCH 01/13] Wrap more deprecated wc.Adm and Context methods Add wc.Adm.entry/entries_read/probe_try/get_prop_diffs/transmit_text_deltas/ transmit_prop_deltas/crawl_revisions and a wc.Entry class, plus wc.Context.get_switch_editor and queue_committed. The transmit_* methods accept the ra commit editor's FileEditor by bridging it across the extension boundary via a PyCapsule. --- subvertpy_util/src/editor.rs | 32 +++ wc/src/adm.rs | 382 +++++++++++++++++++++++++++++++++++ wc/src/context.rs | 149 +++++++++++++- wc/src/lib.rs | 3 +- 4 files changed, 561 insertions(+), 5 deletions(-) diff --git a/subvertpy_util/src/editor.rs b/subvertpy_util/src/editor.rs index fd9204bf..d2786db5 100644 --- a/subvertpy_util/src/editor.rs +++ b/subvertpy_util/src/editor.rs @@ -404,8 +404,40 @@ pub struct PyFileEditor { parent_active_child: Option>>, } +/// PyCapsule name identifying a borrowed `*const WrapFileEditor<'static>`. +/// +/// The `wc` extension module looks up a capsule with this exact name to +/// recover the underlying file editor for the deprecated adm-based +/// transmit_*_deltas functions. Sharing the editor across extension modules +/// via a capsule sidesteps the fact that each cdylib gets its own distinct +/// `FileEditor` Python type object (so PyO3 downcasting across modules fails). +pub const WRAP_FILE_EDITOR_CAPSULE_NAME: &std::ffi::CStr = c"subvertpy._wrap_file_editor"; + #[pymethods] impl PyFileEditor { + /// Return a PyCapsule wrapping a borrowed pointer to the underlying + /// `WrapFileEditor`. + /// + /// This lets other extension modules (notably ``wc``) recover the real + /// delta editor and baton from the commit drive, which is required by the + /// deprecated adm-based ``transmit_*_deltas`` functions. The capsule + /// borrows from this object, so the caller must keep this ``FileEditor`` + /// alive while using the capsule. + fn _wrap_file_editor_capsule<'py>( + &self, + py: Python<'py>, + ) -> PyResult> { + let ptr = &self.editor as *const WrapFileEditor<'static> as *mut std::ffi::c_void; + // SAFETY: `ptr` borrows `self.editor`, which stays valid for as long as + // this PyFileEditor is alive. There is no destructor; ownership remains + // with this object. + let non_null = + std::ptr::NonNull::new(ptr).expect("address of a struct field is never null"); + unsafe { + pyo3::types::PyCapsule::new_with_pointer(py, non_null, WRAP_FILE_EDITOR_CAPSULE_NAME) + } + } + #[pyo3(signature = (base_checksum=None))] fn apply_textdelta( &mut self, diff --git a/wc/src/adm.rs b/wc/src/adm.rs index e012fbbe..08ac1a8c 100644 --- a/wc/src/adm.rs +++ b/wc/src/adm.rs @@ -3,6 +3,226 @@ use pyo3::prelude::*; use subvertpy_util::error::svn_err_to_py; +/// Recover a borrowed [`WrapFileEditor`] from a Python file editor object. +/// +/// The file editor produced by ``ra.RemoteAccess.get_commit_editor`` lives in +/// the ``_ra`` extension module, which has its own ``FileEditor`` Python type +/// distinct from the one in this (``wc``) module, so it cannot be downcast +/// directly. Instead we ask it for a PyCapsule wrapping the underlying +/// ``WrapFileEditor`` pointer (see +/// ``subvertpy_util::editor::PyFileEditor::_wrap_file_editor_capsule``) and +/// borrow that. +/// +/// # Safety +/// +/// The returned reference borrows from the Python file editor object, which the +/// caller (`editor`) keeps alive for the duration of the borrow. The pointers +/// inside belong to the live commit-editor drive, so they line up with the +/// access baton used by the transmit call. +unsafe fn wrap_file_editor_from_py<'a>( + editor: &'a Bound<'_, PyAny>, +) -> PyResult<&'a subversion::delta::WrapFileEditor<'static>> { + let capsule = editor.call_method0("_wrap_file_editor_capsule")?; + let capsule = capsule.cast::().map_err(|_| { + PyErr::new::( + "editor did not return a file editor capsule", + ) + })?; + let name = subvertpy_util::editor::WRAP_FILE_EDITOR_CAPSULE_NAME; + let ptr = capsule.pointer_checked(Some(name))?; + // SAFETY: the capsule pointer is a `*const WrapFileEditor<'static>` produced + // by subvertpy_util, valid while `editor` is alive (no destructor). + Ok(unsafe { &*(ptr.as_ptr() as *const subversion::delta::WrapFileEditor<'static>) }) +} + +/// Convert a subversion::NodeKind to the Python integer constant. +fn node_kind_to_py(kind: subversion::NodeKind) -> i32 { + match kind { + subversion::NodeKind::None => 0, + subversion::NodeKind::File => 1, + subversion::NodeKind::Dir => 2, + subversion::NodeKind::Unknown => 3, + subversion::NodeKind::Symlink => 4, + } +} + +/// A deprecated working copy entry (``svn_wc_entry_t``). +/// +/// All attributes are read-only copies, safe to use after the access baton +/// is closed. +#[pyclass(name = "Entry")] +pub struct Entry { + inner: subversion::wc::adm::Entry, +} + +#[pymethods] +impl Entry { + #[getter] + fn name(&self) -> Option<&str> { + self.inner.name.as_deref() + } + + #[getter] + fn revision(&self) -> i64 { + self.inner.revision.as_i64() + } + + #[getter] + fn url(&self) -> Option<&str> { + self.inner.url.as_deref() + } + + #[getter] + fn repos(&self) -> Option<&str> { + self.inner.repos.as_deref() + } + + #[getter] + fn uuid(&self) -> Option<&str> { + self.inner.uuid.as_deref() + } + + #[getter] + fn kind(&self) -> i32 { + node_kind_to_py(self.inner.kind) + } + + #[getter] + fn schedule(&self) -> u32 { + self.inner.schedule + } + + #[getter] + fn copied(&self) -> bool { + self.inner.copied + } + + #[getter] + fn deleted(&self) -> bool { + self.inner.deleted + } + + #[getter] + fn absent(&self) -> bool { + self.inner.absent + } + + #[getter] + fn incomplete(&self) -> bool { + self.inner.incomplete + } + + #[getter] + fn copyfrom_url(&self) -> Option<&str> { + self.inner.copyfrom_url.as_deref() + } + + #[getter] + fn copyfrom_rev(&self) -> i64 { + self.inner.copyfrom_rev.as_i64() + } + + #[getter] + fn conflict_old(&self) -> Option<&str> { + self.inner.conflict_old.as_deref() + } + + #[getter] + fn conflict_new(&self) -> Option<&str> { + self.inner.conflict_new.as_deref() + } + + #[getter] + fn conflict_wrk(&self) -> Option<&str> { + self.inner.conflict_wrk.as_deref() + } + + #[getter] + fn prejfile(&self) -> Option<&str> { + self.inner.prejfile.as_deref() + } + + #[getter] + fn text_time(&self) -> i64 { + self.inner.text_time + } + + #[getter] + fn prop_time(&self) -> i64 { + self.inner.prop_time + } + + #[getter] + fn checksum(&self) -> Option<&str> { + self.inner.checksum.as_deref() + } + + #[getter] + fn cmt_rev(&self) -> i64 { + self.inner.cmt_rev.as_i64() + } + + #[getter] + fn cmt_date(&self) -> i64 { + self.inner.cmt_date + } + + #[getter] + fn cmt_author(&self) -> Option<&str> { + self.inner.cmt_author.as_deref() + } + + #[getter] + fn lock_token(&self) -> Option<&str> { + self.inner.lock_token.as_deref() + } + + #[getter] + fn lock_owner(&self) -> Option<&str> { + self.inner.lock_owner.as_deref() + } + + #[getter] + fn lock_comment(&self) -> Option<&str> { + self.inner.lock_comment.as_deref() + } + + #[getter] + fn lock_creation_date(&self) -> i64 { + self.inner.lock_creation_date + } + + #[getter] + fn has_props(&self) -> bool { + self.inner.has_props + } + + #[getter] + fn has_prop_mods(&self) -> bool { + self.inner.has_prop_mods + } + + #[getter] + fn changelist(&self) -> Option<&str> { + self.inner.changelist.as_deref() + } + + #[getter] + fn working_size(&self) -> i64 { + self.inner.working_size + } + + #[getter] + fn keep_local(&self) -> bool { + self.inner.keep_local + } + + #[getter] + fn depth(&self) -> i32 { + crate::context::depth_to_py(self.inner.depth) + } +} + /// Deprecated working copy administrative access baton. /// /// Wraps the deprecated ``svn_wc_adm_access_t`` based API. @@ -202,6 +422,168 @@ impl Adm { .map_err(svn_err_to_py) } + /// Get a single entry from the working copy. + /// + /// Returns ``None`` if the path is not versioned. + #[pyo3(signature = (path, show_hidden=false))] + fn entry(&self, path: &Bound, show_hidden: bool) -> PyResult> { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + let entry = self + .inner + .entry(&path_str, show_hidden) + .map_err(svn_err_to_py)?; + Ok(entry.map(|inner| Entry { inner })) + } + + /// Read all entries in this directory, returning a dict of name -> Entry. + #[pyo3(signature = (show_hidden=false))] + fn entries_read(&self, py: Python<'_>, show_hidden: bool) -> PyResult> { + let entries = self + .inner + .entries_read(show_hidden) + .map_err(svn_err_to_py)?; + let dict = pyo3::types::PyDict::new(py); + for (name, inner) in entries { + dict.set_item(name, Py::new(py, Entry { inner })?)?; + } + Ok(dict.unbind()) + } + + /// Try to obtain an access baton for a path, using this baton as parent. + /// + /// Returns ``None`` if the path is not a versioned directory. The returned + /// baton is tied to this baton's lifetime and must not outlive it. + #[pyo3(signature = (path, write_lock=false, levels_to_lock=0))] + fn probe_try( + &mut self, + path: &Bound, + write_lock: bool, + levels_to_lock: i32, + ) -> PyResult> { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + let sub = self + .inner + .probe_try(&path_str, write_lock, levels_to_lock) + .map_err(svn_err_to_py)?; + // SAFETY: the returned Adm is a borrowed baton (it never closes on + // drop, see the crate's probe_try). Its lifetime is tied to `self`. + // The caller must keep this Adm alive. We erase the lifetime to + // 'static to store it in the pyclass. + Ok(sub.map(|adm| Self { + inner: unsafe { + std::mem::transmute::, subversion::wc::Adm<'static>>(adm) + }, + })) + } + + /// Get the property differences between the working copy and base revision. + /// + /// Returns ``(changes, original_props)`` where ``changes`` is a list of + /// ``(name, value)`` tuples and ``original_props`` is a dict or ``None``. + fn get_prop_diffs( + &self, + py: Python<'_>, + path: &Bound, + ) -> PyResult<(Py, Option>)> { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + let (changes, original) = self + .inner + .get_prop_diffs(&path_str) + .map_err(svn_err_to_py)?; + let list = pyo3::types::PyList::empty(py); + for change in changes { + let value: Py = match change.value { + None => py.None(), + Some(v) => pyo3::types::PyBytes::new(py, &v).into_any().unbind(), + }; + list.append((change.name, value))?; + } + let orig = match original { + None => None, + Some(props) => { + let dict = pyo3::types::PyDict::new(py); + for (name, value) in props { + dict.set_item(name, pyo3::types::PyBytes::new(py, &value))?; + } + Some(dict.unbind()) + } + }; + Ok((list.unbind(), orig)) + } + + /// Transmit local text changes through a file delta editor. + /// + /// Returns ``(tempfile, digest)`` where ``digest`` is the 16-byte MD5 of + /// the transmitted fulltext. + fn transmit_text_deltas( + &self, + py: Python<'_>, + path: &Bound, + fulltext: bool, + editor: &Bound, + ) -> PyResult<(Option, Py)> { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + // SAFETY: the editor object is alive for the duration of this call. + let file_editor = unsafe { wrap_file_editor_from_py(editor)? }; + let (tempfile, digest) = self + .inner + .transmit_text_deltas(&path_str, fulltext, file_editor) + .map_err(svn_err_to_py)?; + Ok((tempfile, pyo3::types::PyBytes::new(py, &digest).unbind())) + } + + /// Transmit local property changes through a file delta editor. + /// + /// Looks up the entry for ``path`` internally. Returns the temporary file + /// path used, if any. + fn transmit_prop_deltas( + &self, + path: &Bound, + editor: &Bound, + ) -> PyResult> { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + // SAFETY: the editor object is alive for the duration of this call. + let file_editor = unsafe { wrap_file_editor_from_py(editor)? }; + self.inner + .transmit_prop_deltas(&path_str, file_editor) + .map_err(svn_err_to_py) + } + + /// Crawl working copy revisions, reporting to a reporter object. + /// + /// ``reporter`` must provide ``set_path``, ``delete_path``, ``link_path``, + /// ``finish`` and ``abort`` methods. + #[pyo3(signature = (path, reporter, restore_files=true, depth=3, honor_depth_exclude=true, depth_compatibility_trick=false, use_commit_times=false, notify_func=None))] + #[allow(clippy::too_many_arguments)] + fn crawl_revisions( + &self, + path: &Bound, + reporter: Py, + restore_files: bool, + depth: i32, + honor_depth_exclude: bool, + depth_compatibility_trick: bool, + use_commit_times: bool, + notify_func: Option>, + ) -> PyResult<()> { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + let py_reporter = crate::context::PyReporterBridge { reporter }; + let wrap_reporter = subversion::ra::WrapReporter::from_rust_reporter(py_reporter); + let notify_fn = crate::context::make_notify_closure(notify_func); + self.inner + .crawl_revisions( + &path_str, + &wrap_reporter, + restore_files, + crate::context::depth_from_py(depth), + honor_depth_exclude, + depth_compatibility_trick, + use_commit_times, + notify_fn.as_deref(), + ) + .map_err(svn_err_to_py) + } + fn __enter__(slf: PyRef<'_, Self>) -> PyRef<'_, Self> { slf } diff --git a/wc/src/context.rs b/wc/src/context.rs index 67f03ef4..521c2388 100644 --- a/wc/src/context.rs +++ b/wc/src/context.rs @@ -3,7 +3,7 @@ use pyo3::prelude::*; use subvertpy_util::error::svn_err_to_py; -fn depth_to_py(depth: subversion::Depth) -> i32 { +pub(crate) fn depth_to_py(depth: subversion::Depth) -> i32 { match depth { subversion::Depth::Unknown => -2, subversion::Depth::Exclude => -1, @@ -29,7 +29,7 @@ pub(crate) fn depth_from_py(depth: i32) -> subversion::Depth { /// /// The Python callback receives the SVN error as an exception object /// when the notification indicates an error. -fn make_notify_closure( +pub(crate) fn make_notify_closure( py_notify: Option>, ) -> Option> { py_notify.map(|py_func| -> Box { @@ -578,6 +578,147 @@ impl Context { parent, )) } + + /// Get an editor for switching the working copy to a different URL. + #[pyo3(signature = ( + anchor_abspath, target_basename, switch_url, use_commit_times=false, depth=3, + depth_is_sticky=false, allow_unver_obstructions=true, + server_performs_filtering=false, diff3_cmd=None, preserved_exts=None, + dirents_func=None, conflict_func=None, external_func=None, notify_func=None + ))] + fn get_switch_editor( + slf: &Bound, + anchor_abspath: &Bound, + target_basename: &str, + switch_url: &str, + use_commit_times: bool, + depth: i32, + depth_is_sticky: bool, + allow_unver_obstructions: bool, + server_performs_filtering: bool, + diff3_cmd: Option<&str>, + preserved_exts: Option>, + dirents_func: Option>, + conflict_func: Option>, + external_func: Option>, + notify_func: Option>, + ) -> PyResult { + if conflict_func.is_some() { + return Err(pyo3::exceptions::PyNotImplementedError::new_err( + "conflict_func is not currently supported", + )); + } + if external_func.is_some() { + return Err(pyo3::exceptions::PyNotImplementedError::new_err( + "external_func is not currently supported", + )); + } + if dirents_func.is_some() { + return Err(pyo3::exceptions::PyNotImplementedError::new_err( + "dirents_func is not currently supported", + )); + } + + let path_str = subvertpy_util::py_to_svn_abspath(anchor_abspath)?; + let switch_url = subversion::uri::canonicalize_uri(switch_url).map_err(svn_err_to_py)?; + + let ext_refs: Vec<&str> = preserved_exts + .as_ref() + .map(|v| v.iter().map(|s| s.as_str()).collect()) + .unwrap_or_default(); + + let options = subversion::wc::SwitchEditorOptions { + use_commit_times, + depth: depth_from_py(depth), + depth_is_sticky, + allow_unver_obstructions, + server_performs_filtering, + diff3_cmd, + preserved_exts: ext_refs, + fetch_dirents_func: None, + conflict_func: None, + external_func: None, + cancel_func: None, + notify_func: make_notify_closure(notify_func), + }; + + let mut this = slf.borrow_mut(); + let (editor, _target_rev) = this + .inner + .get_switch_editor(&path_str, target_basename, &switch_url, options) + .map_err(svn_err_to_py)?; + + let parent = slf.clone().into_any().unbind(); + Ok(subvertpy_util::editor::PyEditor::new_with_parent( + editor, parent, + )) + } + + /// Queue committed items for post-commit processing against this context. + /// + /// This is the Context-level analogue of ``Adm.queue_committed``. The + /// queued items are later applied with :meth:`process_committed_queue`. + #[pyo3(signature = (path, queue, recurse=false, is_committed=true, wcprop_changes=None, remove_lock=false, remove_changelist=false, md5_digest=None, sha1_digest=None))] + fn queue_committed( + &mut self, + path: &Bound, + queue: &mut crate::committed::CommittedQueue, + recurse: bool, + is_committed: bool, + wcprop_changes: Option<&Bound>, + remove_lock: bool, + remove_changelist: bool, + md5_digest: Option<&[u8]>, + sha1_digest: Option<&[u8]>, + ) -> PyResult<()> { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + let path = std::path::Path::new(&path_str); + + let prop_changes = if let Some(dict) = wcprop_changes { + let dict: &Bound = dict.cast()?; + let mut changes = Vec::with_capacity(dict.len()); + for (key, val) in dict.iter() { + let name: String = key.extract()?; + let value: Option> = if val.is_none() { + None + } else { + Some(val.extract::>()?) + }; + changes.push(subversion::wc::PropChange { name, value }); + } + Some(changes) + } else { + None + }; + + let pool = apr::Pool::new(); + let checksum = if let Some(digest) = sha1_digest { + Some( + subversion::Checksum::from_digest(subversion::ChecksumKind::SHA1, digest, &pool) + .map_err(svn_err_to_py)?, + ) + } else if let Some(digest) = md5_digest { + Some( + subversion::Checksum::from_digest(subversion::ChecksumKind::MD5, digest, &pool) + .map_err(svn_err_to_py)?, + ) + } else { + None + }; + + self.inner + .queue_committed( + path, + recurse, + is_committed, + &mut queue.inner, + prop_changes.as_deref(), + remove_lock, + remove_changelist, + checksum.as_ref(), + ) + .map_err(svn_err_to_py) + } } /// Bridge between a Python reporter object and the Rust Reporter trait. @@ -588,8 +729,8 @@ impl Context { /// - link_path(path, url, revision, start_empty, lock_token, depth) /// - finish() /// - abort() -struct PyReporterBridge { - reporter: Py, +pub(crate) struct PyReporterBridge { + pub(crate) reporter: Py, } impl subversion::ra::Reporter for PyReporterBridge { diff --git a/wc/src/lib.rs b/wc/src/lib.rs index 14f6ec86..f6c92100 100644 --- a/wc/src/lib.rs +++ b/wc/src/lib.rs @@ -11,7 +11,7 @@ mod context; mod lock; mod status; -use adm::Adm; +use adm::{Adm, Entry}; use committed::CommittedQueue; use context::Context; use lock::Lock; @@ -187,6 +187,7 @@ fn revision_status( #[pymodule] fn wc(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_class::()?; + m.add_class::()?; m.add_class::()?; m.add_class::()?; m.add_class::()?; From ef162184923e74c78117c7bcf7d9c4165e604245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Mon, 22 Jun 2026 13:37:51 +0100 Subject: [PATCH 02/13] Wrap Context write-lock and pristine-install methods Add wc.Context.acquire_write_lock / release_write_lock / install_text_base so the working-tree commit-finalization path can install pristines and bump node base revisions. Patch the subversion dependency to the local subversion-rs checkout that provides these. --- Cargo.lock | 8 ++++++++ Cargo.toml | 6 ++++++ wc/src/context.rs | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 168744e0..96e492e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -617,3 +617,11 @@ name = "winnow" version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0592e1c9d151f854e6fd382574c3a0855250e1d9b2f99d9281c6e6391af352f1" + +[[patch.unused]] +name = "subversion" +version = "0.1.11" + +[[patch.unused]] +name = "subversion-sys" +version = "0.1.2" diff --git a/Cargo.toml b/Cargo.toml index d2ce7936..27bfa125 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,3 +12,9 @@ pyo3 = { version = "0.29" } subversion = { version = "0.1.12" } #subversion = { path = "../subversion-rs" } pyo3-filelike = { version = "0.5" } + +# Use the local subversion-rs checkout that adds Context write-lock support. +# Drop this once those changes are released to crates.io. +[patch.crates-io] +subversion = { path = "../subversion-rs" } +subversion-sys = { path = "../subversion-rs/subversion-sys" } diff --git a/wc/src/context.rs b/wc/src/context.rs index 521c2388..0382321d 100644 --- a/wc/src/context.rs +++ b/wc/src/context.rs @@ -719,6 +719,45 @@ impl Context { ) .map_err(svn_err_to_py) } + + /// Acquire a working copy write lock. + /// + /// Required before processing a committed queue. Returns the abspath of + /// the lock root, which must be passed to :meth:`release_write_lock`. + #[pyo3(signature = (path, lock_anchor=false))] + fn acquire_write_lock(&mut self, path: &Bound, lock_anchor: bool) -> PyResult { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + let root = self + .inner + .acquire_write_lock(std::path::Path::new(&path_str), lock_anchor) + .map_err(svn_err_to_py)?; + Ok(root.to_string_lossy().into_owned()) + } + + /// Release a working copy write lock acquired with + /// :meth:`acquire_write_lock`. + /// + /// :param path: The lock root abspath returned by acquire_write_lock. + fn release_write_lock(&mut self, path: &Bound) -> PyResult<()> { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + self.inner + .release_write_lock(std::path::Path::new(&path_str)) + .map_err(svn_err_to_py) + } + + /// Install a working file's current text into the pristine store. + /// + /// Needed before :meth:`queue_committed` / :meth:`process_committed_queue` + /// can bump a file whose commit was performed outside this working copy. + /// Returns the SHA-1 checksum (hex) of the installed pristine. + /// + /// :param path: Absolute path to the working file. + /// :param basename: Entry name of the file within its parent directory. + fn install_text_base(&mut self, path: &Bound, basename: &str) -> PyResult { + let path_str = subvertpy_util::py_to_svn_abspath(path)?; + subversion::wc::install_text_base(&mut self.inner, &path_str, basename) + .map_err(svn_err_to_py) + } } /// Bridge between a Python reporter object and the Rust Reporter trait. From c5a705ae4109080d603852746d3e7e71f8d39a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Mon, 22 Jun 2026 15:30:48 +0100 Subject: [PATCH 03/13] macos-wheels: Add 45-minute job timeout --- .github/workflows/macos-wheels.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/macos-wheels.yml b/.github/workflows/macos-wheels.yml index b7d6b008..0cb43092 100644 --- a/.github/workflows/macos-wheels.yml +++ b/.github/workflows/macos-wheels.yml @@ -5,6 +5,7 @@ jobs: name: subvertpy wheel build for Python ${{ matrix.python-version }} on ${{ matrix.config.os }} runs-on: ${{ matrix.config.os }} + timeout-minutes: 45 strategy: fail-fast: false matrix: From dd62c8b55474c6daaae21d05dfc84ad257b0ba61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Mon, 22 Jun 2026 18:11:50 +0100 Subject: [PATCH 04/13] Abort the edit and release the session on editor __exit__ error PyEditor.__exit__ always called close(), so a failed commit left the editor unclosed and the RA session permanently busy. Abort when an exception is propagating, and run the on_close callback (which clears the session busy flag) even if close/abort itself fails. --- subvertpy_util/src/editor.rs | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/subvertpy_util/src/editor.rs b/subvertpy_util/src/editor.rs index d2786db5..ae57af14 100644 --- a/subvertpy_util/src/editor.rs +++ b/subvertpy_util/src/editor.rs @@ -112,12 +112,12 @@ impl PyEditor { .close() .map_err(|e| crate::error::svn_err_to_py(e)); - if result.is_ok() { - self.closed = true; - // Call on_close callback if set - if let Some(callback) = self.on_close.take() { - callback(); - } + // The edit is over either way: mark closed and release resources (e.g. + // the session busy flag) even if close itself failed, otherwise the + // session stays permanently busy. + self.closed = true; + if let Some(callback) = self.on_close.take() { + callback(); } result @@ -135,12 +135,9 @@ impl PyEditor { .abort() .map_err(|e| crate::error::svn_err_to_py(e)); - if result.is_ok() { - self.closed = true; - // Call on_close callback if set - if let Some(callback) = self.on_close.take() { - callback(); - } + self.closed = true; + if let Some(callback) = self.on_close.take() { + callback(); } result @@ -153,11 +150,21 @@ impl PyEditor { fn __exit__( &mut self, py: Python, - _exc_type: Bound, + exc_type: Bound, _exc_value: Bound, _traceback: Bound, ) -> PyResult { - self.close(py)?; + if self.closed { + return Ok(false); + } + // On a normal exit close the edit; if an exception is propagating, + // abort instead so the editor (and the session it holds busy) is + // released even though the commit did not complete. + if exc_type.is_none() { + self.close(py)?; + } else { + self.abort(py)?; + } Ok(false) } } From e8c35853b1795667742dc1fe676837d019ccb77b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Mon, 22 Jun 2026 18:18:08 +0100 Subject: [PATCH 05/13] Return parsed mergeinfo ranges from RemoteAccess.mergeinfo --- ra/src/session.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/ra/src/session.rs b/ra/src/session.rs index 47e9afb2..0b0c4111 100644 --- a/ra/src/session.rs +++ b/ra/src/session.rs @@ -1337,8 +1337,17 @@ impl RemoteAccess { Python::attach(|py| { let dict = PyDict::new(py); for (path, mergeinfo) in &result { - let mi_str = mergeinfo.to_string().map_err(|e| svn_err_to_py(e))?; - dict.set_item(path, mi_str)?; + let inner = PyDict::new(py); + for (mi_path, ranges) in mergeinfo.paths_with_ranges() { + let list = pyo3::types::PyList::empty(py); + for r in ranges { + let tuple = + (r.start.as_i64(), r.end.as_i64(), if r.inheritable { 1 } else { 0 }); + list.append(tuple)?; + } + inner.set_item(mi_path, list)?; + } + dict.set_item(path, inner)?; } Ok(Some(dict.into_any().unbind())) }) From 9a8e887e4ea9982b4b78c47f2aff44ad70453ede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Mon, 22 Jun 2026 20:32:00 +0100 Subject: [PATCH 06/13] Wrap Adm.get_switch_editor --- wc/src/adm.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/wc/src/adm.rs b/wc/src/adm.rs index 08ac1a8c..7baba649 100644 --- a/wc/src/adm.rs +++ b/wc/src/adm.rs @@ -584,6 +584,46 @@ impl Adm { .map_err(svn_err_to_py) } + /// Get an editor for switching this working copy to a different URL. + /// + /// Anchored on this (deprecated) access baton, which holds the lock. + #[pyo3(signature = (target, switch_url, use_commit_times=false, depth=3, + notify_func=None, diff3_cmd=None, depth_is_sticky=false, + allow_unver_obstructions=true))] + #[allow(clippy::too_many_arguments)] + fn get_switch_editor( + slf: &Bound, + target: &str, + switch_url: &str, + use_commit_times: bool, + depth: i32, + notify_func: Option>, + diff3_cmd: Option<&str>, + depth_is_sticky: bool, + allow_unver_obstructions: bool, + ) -> PyResult { + let _ = notify_func; + let switch_url = subversion::uri::canonicalize_uri(switch_url).map_err(svn_err_to_py)?; + let this = slf.borrow(); + let (editor, _target_rev) = this + .inner + .get_switch_editor( + target, + &switch_url, + use_commit_times, + crate::context::depth_from_py(depth), + depth_is_sticky, + allow_unver_obstructions, + diff3_cmd, + ) + .map_err(svn_err_to_py)?; + drop(this); + let parent = slf.clone().into_any().unbind(); + Ok(subvertpy_util::editor::PyEditor::new_with_parent( + editor, parent, + )) + } + fn __enter__(slf: PyRef<'_, Self>) -> PyRef<'_, Self> { slf } From 761b30d1b74022f4a9c321209137a49fd4c64145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Mon, 22 Jun 2026 20:34:06 +0100 Subject: [PATCH 07/13] Format mergeinfo tuple construction --- ra/src/session.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ra/src/session.rs b/ra/src/session.rs index 0b0c4111..ec47783f 100644 --- a/ra/src/session.rs +++ b/ra/src/session.rs @@ -1341,8 +1341,11 @@ impl RemoteAccess { for (mi_path, ranges) in mergeinfo.paths_with_ranges() { let list = pyo3::types::PyList::empty(py); for r in ranges { - let tuple = - (r.start.as_i64(), r.end.as_i64(), if r.inheritable { 1 } else { 0 }); + let tuple = ( + r.start.as_i64(), + r.end.as_i64(), + if r.inheritable { 1 } else { 0 }, + ); list.append(tuple)?; } inner.set_item(mi_path, list)?; From 79bf56f5acaceba812967de3a6bee3cb83c38fac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Tue, 23 Jun 2026 00:24:15 +0100 Subject: [PATCH 08/13] Expose ERR_WC_CORRUPT --- subvertpy/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/subvertpy/__init__.py b/subvertpy/__init__.py index a07c8d05..368b0446 100644 --- a/subvertpy/__init__.py +++ b/subvertpy/__init__.py @@ -53,6 +53,7 @@ ERR_WC_NOT_WORKING_COPY = ERR_WC_NOT_DIRECTORY = 155007 ERR_ENTRY_EXISTS = 150002 ERR_WC_PATH_NOT_FOUND = 155010 +ERR_WC_CORRUPT = 155016 ERR_WC_PATH_UNEXPECTED_STATUS = 155035 ERR_CANCELLED = 200015 ERR_WC_UNSUPPORTED_FORMAT = 155021 From 0cd94c6b56de8cd5730ea63fa0103b59080c781d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Tue, 23 Jun 2026 00:40:33 +0100 Subject: [PATCH 09/13] Drive switch/update editor and reporter directly across modules When wc.Context.get_switch_editor's editor and a do_switch reporter are passed back through do_switch/crawl_revisions, recover the underlying WrapEditor and raw SVN reporter via PyCapsules instead of bouncing every callback through Python, which recursively re-entered the interpreter and corrupted the edit. --- ra/src/py_editor.rs | 21 ++++++++ ra/src/reporter.rs | 68 +++++++++++++++++++++++++ ra/src/session.rs | 47 +++++++++++++++--- subvertpy_util/src/editor.rs | 21 ++++++++ wc/src/context.rs | 96 +++++++++++++++++++++++++++++++++++- 5 files changed, 245 insertions(+), 8 deletions(-) diff --git a/ra/src/py_editor.rs b/ra/src/py_editor.rs index da58ff7f..8f01c306 100644 --- a/ra/src/py_editor.rs +++ b/ra/src/py_editor.rs @@ -8,6 +8,27 @@ use pyo3::types::PyBytes; use subversion::delta::{DirectoryEditor, Editor, FileEditor}; use subvertpy_util::error::py_err_to_svn; +/// If `editor` is a subvertpy editor exposing a wrap-editor capsule, return a +/// mutable reference to the underlying `WrapEditor`. Returns `None` for any +/// other Python object. +/// +/// The reference borrows from the live Python editor object; the caller must +/// keep that object alive for as long as the reference is used. +pub(crate) fn wrap_editor_capsule( + py: Python<'_>, + editor: &Py, +) -> Option<&'static mut subversion::delta::WrapEditor<'static>> { + let bound = editor.bind(py); + let capsule = bound.call_method0("_wrap_editor_capsule").ok()?; + let capsule = capsule.cast::().ok()?; + let name = subvertpy_util::editor::WRAP_EDITOR_CAPSULE_NAME; + let ptr = capsule.pointer_checked(Some(name)).ok()?; + // SAFETY: the capsule pointer is a `*mut WrapEditor<'static>` produced by + // subvertpy_util, valid while the Python editor is alive (no destructor). + // do_switch/do_update only read it via as_raw_parts. + Some(unsafe { &mut *(ptr.as_ptr() as *mut subversion::delta::WrapEditor<'static>) }) +} + /// Wrapper for a Python editor object pub struct PyEditorWrapper { py_editor: Py, diff --git a/ra/src/reporter.rs b/ra/src/reporter.rs index 1d560376..b37b5309 100644 --- a/ra/src/reporter.rs +++ b/ra/src/reporter.rs @@ -15,6 +15,19 @@ pub struct Reporter { _session: Option>, /// Keep the editor alive for do_diff (reporter has C pointers to it) _editor: Option>>, + /// Keep an arbitrary Python object alive (e.g. a subvertpy editor whose + /// WrapEditor is borrowed by the reporter through a capsule). + _keepalive: Option>, + /// Heap box backing a raw-reporter capsule handed out to the wc module. + _raw_reporter_box: Option<*mut (*const std::ffi::c_void, *mut std::ffi::c_void)>, +} + +impl Drop for Reporter { + fn drop(&mut self) { + if let Some(raw) = self._raw_reporter_box.take() { + unsafe { drop(Box::from_raw(raw)) }; + } + } } impl Reporter { @@ -24,6 +37,8 @@ impl Reporter { finished: false, _session: None, _editor: None, + _keepalive: None, + _raw_reporter_box: None, } } @@ -36,6 +51,8 @@ impl Reporter { finished: false, _session: Some(session), _editor: None, + _keepalive: None, + _raw_reporter_box: None, } } @@ -49,12 +66,63 @@ impl Reporter { finished: false, _session: Some(session), _editor: Some(editor), + _keepalive: None, + _raw_reporter_box: None, + } + } + + pub fn new_with_session_and_keepalive( + reporter: Box, + session: Py, + keepalive: Py, + ) -> Self { + Self { + reporter: Some(reporter), + finished: false, + _session: Some(session), + _editor: None, + _keepalive: Some(keepalive), + _raw_reporter_box: None, } } } +/// PyCapsule name identifying a borrowed raw SVN reporter. +/// +/// Wraps a pointer to a heap `(svn_ra_reporter3_t*, baton)` pair so the ``wc`` +/// extension can drive a crawl directly against this reporter instead of +/// bouncing every callback back through Python. +pub const RAW_REPORTER_CAPSULE_NAME: &std::ffi::CStr = c"subvertpy._raw_reporter"; + #[pymethods] impl Reporter { + /// Return a PyCapsule with a borrowed pointer to the raw SVN reporter, or + /// None if this reporter is not backed by one. + fn _raw_reporter_capsule<'py>( + &mut self, + py: Python<'py>, + ) -> PyResult>> { + let parts = match self.reporter.as_ref() { + Some(r) => r.as_raw_reporter(), + None => None, + }; + let Some((ptr, baton)) = parts else { + return Ok(None); + }; + // Box the pair and hand the capsule a borrowed pointer to it. The + // pointers borrow from `self.reporter`, which the caller must keep + // alive; we leak the small box (freed when the reporter is dropped via + // _raw_reporter_box below) to avoid Send requirements on PyCapsule::new. + let boxed: Box<(*const std::ffi::c_void, *mut std::ffi::c_void)> = Box::new((ptr, baton)); + let raw = Box::into_raw(boxed); + self._raw_reporter_box = Some(raw); + let non_null = std::ptr::NonNull::new(raw as *mut std::ffi::c_void).unwrap(); + let capsule = unsafe { + pyo3::types::PyCapsule::new_with_pointer(py, non_null, RAW_REPORTER_CAPSULE_NAME)? + }; + Ok(Some(capsule)) + } + /// Set a path #[pyo3(signature = (path, revision, start_empty, lock_token=None, depth=None))] fn set_path( diff --git a/ra/src/session.rs b/ra/src/session.rs index ec47783f..a54fb87f 100644 --- a/ra/src/session.rs +++ b/ra/src/session.rs @@ -1142,18 +1142,53 @@ impl RemoteAccess { let rev = subvertpy_util::to_revnum_or_head(revision); let switch_target = subvertpy_util::to_relpath(switch_target)?; - let py_editor = crate::py_editor::PyEditorWrapper::new(switch_editor); - let wrap_editor = py_editor.into_wrap_editor(); - - let boxed_editor = Box::new(wrap_editor); - let editor_ptr: *mut subversion::delta::WrapEditor = Box::into_raw(boxed_editor); - let depth = if recurse { subversion::Depth::Infinity } else { subversion::Depth::Files }; + // If the editor is itself a subvertpy editor (e.g. from + // wc.Context.get_switch_editor), drive its real delta editor directly + // through a capsule rather than bouncing every callback back through + // Python, which would re-enter the interpreter recursively. + let editor_capsule = crate::py_editor::wrap_editor_capsule(slf.py(), &switch_editor); + + if let Some(editor_ref) = editor_capsule { + let reporter = unsafe { + let session_ptr = + &mut slf.borrow_mut().session as *mut subversion::ra::Session<'static>; + let raw_reporter = (*session_ptr) + .do_switch( + rev, + switch_target.as_str(), + depth, + switch_url, + send_copyfrom_args, + ignore_ancestry, + editor_ref, + ) + .map_err(|e| svn_err_to_py(e))?; + std::mem::transmute::< + Box, + Box, + >(raw_reporter) + }; + // Keep the Python editor alive for as long as the reporter, so the + // capsule pointer stays valid. + return Ok(crate::reporter::Reporter::new_with_session_and_keepalive( + reporter, + slf.unbind().into_any(), + switch_editor, + )); + } + + let py_editor = crate::py_editor::PyEditorWrapper::new(switch_editor); + let wrap_editor = py_editor.into_wrap_editor(); + + let boxed_editor = Box::new(wrap_editor); + let editor_ptr: *mut subversion::delta::WrapEditor = Box::into_raw(boxed_editor); + let reporter = unsafe { let editor_ref = &mut *editor_ptr; diff --git a/subvertpy_util/src/editor.rs b/subvertpy_util/src/editor.rs index ae57af14..73850791 100644 --- a/subvertpy_util/src/editor.rs +++ b/subvertpy_util/src/editor.rs @@ -70,6 +70,24 @@ impl PyEditor { #[pymethods] impl PyEditor { + /// Return a PyCapsule wrapping a borrowed pointer to the underlying + /// `WrapEditor`. + /// + /// This lets the ``ra`` extension module recover the real delta editor when + /// a subvertpy editor (e.g. from ``wc.Context.get_switch_editor``) is handed + /// to ``do_switch`` / ``do_update``, instead of bouncing every callback back + /// through Python. The capsule borrows from this object, so the caller must + /// keep this ``Editor`` alive while using the capsule. + fn _wrap_editor_capsule<'py>( + &self, + py: Python<'py>, + ) -> PyResult> { + let ptr = &self.editor as *const WrapEditor<'static> as *mut std::ffi::c_void; + let non_null = + std::ptr::NonNull::new(ptr).expect("address of a struct field is never null"); + unsafe { pyo3::types::PyCapsule::new_with_pointer(py, non_null, WRAP_EDITOR_CAPSULE_NAME) } + } + fn set_target_revision(&mut self, _py: Python, revision: Option) -> PyResult<()> { let revnum = revision.and_then(|r| to_revnum(r)); self.editor @@ -420,6 +438,9 @@ pub struct PyFileEditor { /// `FileEditor` Python type object (so PyO3 downcasting across modules fails). pub const WRAP_FILE_EDITOR_CAPSULE_NAME: &std::ffi::CStr = c"subvertpy._wrap_file_editor"; +/// PyCapsule name identifying a borrowed `*const WrapEditor<'static>`. +pub const WRAP_EDITOR_CAPSULE_NAME: &std::ffi::CStr = c"subvertpy._wrap_editor"; + #[pymethods] impl PyFileEditor { /// Return a PyCapsule wrapping a borrowed pointer to the underlying diff --git a/wc/src/context.rs b/wc/src/context.rs index 0382321d..08fbe394 100644 --- a/wc/src/context.rs +++ b/wc/src/context.rs @@ -3,6 +3,78 @@ use pyo3::prelude::*; use subvertpy_util::error::svn_err_to_py; +/// Recover a raw SVN reporter (vtable + baton) from a subvertpy reporter +/// object via its capsule. Returns None for any other object. +fn raw_reporter_from_py( + py: Python<'_>, + reporter: &Py, +) -> PyResult> { + let bound = reporter.bind(py); + let capsule = match bound.call_method0("_raw_reporter_capsule") { + Ok(c) => c, + Err(_) => return Ok(None), + }; + if capsule.is_none() { + return Ok(None); + } + let capsule = match capsule.cast::() { + Ok(c) => c, + Err(_) => return Ok(None), + }; + let name = c"subvertpy._raw_reporter"; + let ptr = capsule.pointer_checked(Some(name))?; + // SAFETY: the capsule points to a heap (vtable, baton) pair owned by the + // reporter object, valid while that object is alive. + let pair = + unsafe { *(ptr.as_ptr() as *const (*const std::ffi::c_void, *mut std::ffi::c_void)) }; + Ok(Some(pair)) +} + +/// Adapter exposing a raw SVN reporter to `crawl_revisions5`. Only +/// `as_raw_reporter` is ever called; the trait methods are unreachable here. +struct RawReporter((*const std::ffi::c_void, *mut std::ffi::c_void)); + +impl subversion::ra::Reporter for RawReporter { + fn set_path( + &mut self, + _path: &str, + _rev: subversion::Revnum, + _depth: subversion::Depth, + _start_empty: bool, + _lock_token: &str, + ) -> Result<(), subversion::Error<'static>> { + unreachable!("RawReporter is only used for as_raw_reporter") + } + + fn delete_path(&mut self, _path: &str) -> Result<(), subversion::Error<'static>> { + unreachable!("RawReporter is only used for as_raw_reporter") + } + + fn link_path( + &mut self, + _path: &str, + _url: &str, + _rev: subversion::Revnum, + _depth: subversion::Depth, + _start_empty: bool, + _lock_token: &str, + ) -> Result<(), subversion::Error<'static>> { + unreachable!("RawReporter is only used for as_raw_reporter") + } + + fn finish_report(&mut self) -> Result<(), subversion::Error<'static>> { + unreachable!("RawReporter is only used for as_raw_reporter") + } + + fn abort_report(&mut self) -> Result<(), subversion::Error<'static>> { + unreachable!("RawReporter is only used for as_raw_reporter") + } + + fn as_raw_reporter(&self) -> Option<(*const std::ffi::c_void, *mut std::ffi::c_void)> { + Some(self.0) + } +} + pub(crate) fn depth_to_py(depth: subversion::Depth) -> i32 { match depth { subversion::Depth::Unknown => -2, @@ -448,7 +520,7 @@ impl Context { #[pyo3(signature = (path, reporter, restore_files=true, depth=3, honor_depth_exclude=true, depth_compatibility_trick=false, use_commit_times=false, notify=None))] fn crawl_revisions( &mut self, - _py: Python, + py: Python, path: &Bound, reporter: Py, restore_files: bool, @@ -459,11 +531,31 @@ impl Context { notify: Option>, ) -> PyResult<()> { let path_str = subvertpy_util::py_to_svn_abspath(path)?; + let notify_fn = make_notify_closure(notify); + + // If the reporter is a subvertpy do_update/do_switch reporter, drive the + // crawl against its raw SVN reporter directly. Bouncing each crawl + // callback back through Python (PyReporterBridge) re-enters the + // interpreter recursively and corrupts the in-progress edit. + if let Some(raw) = raw_reporter_from_py(py, &reporter)? { + let mut adapter = RawReporter(raw); + return subversion::wc::crawl_revisions5( + &mut self.inner, + &path_str, + &mut adapter, + restore_files, + depth_from_py(depth), + honor_depth_exclude, + depth_compatibility_trick, + use_commit_times, + notify_fn.as_deref(), + ) + .map_err(svn_err_to_py); + } let py_reporter = PyReporterBridge { reporter }; let mut wrap_reporter = subversion::ra::WrapReporter::from_rust_reporter(py_reporter); - let notify_fn = make_notify_closure(notify); subversion::wc::crawl_revisions5( &mut self.inner, &path_str, From 0dfd7d6881963de821d228ebf6b9b36f32de9092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Tue, 23 Jun 2026 00:47:51 +0100 Subject: [PATCH 10/13] Revert "Drive switch/update editor and reporter directly across modules" This reverts commit ad2f0c799dd7b061920e6af9a2a2f1b7417a5836. --- ra/src/py_editor.rs | 21 -------- ra/src/reporter.rs | 68 ------------------------- ra/src/session.rs | 47 +++--------------- subvertpy_util/src/editor.rs | 21 -------- wc/src/context.rs | 96 +----------------------------------- 5 files changed, 8 insertions(+), 245 deletions(-) diff --git a/ra/src/py_editor.rs b/ra/src/py_editor.rs index 8f01c306..da58ff7f 100644 --- a/ra/src/py_editor.rs +++ b/ra/src/py_editor.rs @@ -8,27 +8,6 @@ use pyo3::types::PyBytes; use subversion::delta::{DirectoryEditor, Editor, FileEditor}; use subvertpy_util::error::py_err_to_svn; -/// If `editor` is a subvertpy editor exposing a wrap-editor capsule, return a -/// mutable reference to the underlying `WrapEditor`. Returns `None` for any -/// other Python object. -/// -/// The reference borrows from the live Python editor object; the caller must -/// keep that object alive for as long as the reference is used. -pub(crate) fn wrap_editor_capsule( - py: Python<'_>, - editor: &Py, -) -> Option<&'static mut subversion::delta::WrapEditor<'static>> { - let bound = editor.bind(py); - let capsule = bound.call_method0("_wrap_editor_capsule").ok()?; - let capsule = capsule.cast::().ok()?; - let name = subvertpy_util::editor::WRAP_EDITOR_CAPSULE_NAME; - let ptr = capsule.pointer_checked(Some(name)).ok()?; - // SAFETY: the capsule pointer is a `*mut WrapEditor<'static>` produced by - // subvertpy_util, valid while the Python editor is alive (no destructor). - // do_switch/do_update only read it via as_raw_parts. - Some(unsafe { &mut *(ptr.as_ptr() as *mut subversion::delta::WrapEditor<'static>) }) -} - /// Wrapper for a Python editor object pub struct PyEditorWrapper { py_editor: Py, diff --git a/ra/src/reporter.rs b/ra/src/reporter.rs index b37b5309..1d560376 100644 --- a/ra/src/reporter.rs +++ b/ra/src/reporter.rs @@ -15,19 +15,6 @@ pub struct Reporter { _session: Option>, /// Keep the editor alive for do_diff (reporter has C pointers to it) _editor: Option>>, - /// Keep an arbitrary Python object alive (e.g. a subvertpy editor whose - /// WrapEditor is borrowed by the reporter through a capsule). - _keepalive: Option>, - /// Heap box backing a raw-reporter capsule handed out to the wc module. - _raw_reporter_box: Option<*mut (*const std::ffi::c_void, *mut std::ffi::c_void)>, -} - -impl Drop for Reporter { - fn drop(&mut self) { - if let Some(raw) = self._raw_reporter_box.take() { - unsafe { drop(Box::from_raw(raw)) }; - } - } } impl Reporter { @@ -37,8 +24,6 @@ impl Reporter { finished: false, _session: None, _editor: None, - _keepalive: None, - _raw_reporter_box: None, } } @@ -51,8 +36,6 @@ impl Reporter { finished: false, _session: Some(session), _editor: None, - _keepalive: None, - _raw_reporter_box: None, } } @@ -66,63 +49,12 @@ impl Reporter { finished: false, _session: Some(session), _editor: Some(editor), - _keepalive: None, - _raw_reporter_box: None, - } - } - - pub fn new_with_session_and_keepalive( - reporter: Box, - session: Py, - keepalive: Py, - ) -> Self { - Self { - reporter: Some(reporter), - finished: false, - _session: Some(session), - _editor: None, - _keepalive: Some(keepalive), - _raw_reporter_box: None, } } } -/// PyCapsule name identifying a borrowed raw SVN reporter. -/// -/// Wraps a pointer to a heap `(svn_ra_reporter3_t*, baton)` pair so the ``wc`` -/// extension can drive a crawl directly against this reporter instead of -/// bouncing every callback back through Python. -pub const RAW_REPORTER_CAPSULE_NAME: &std::ffi::CStr = c"subvertpy._raw_reporter"; - #[pymethods] impl Reporter { - /// Return a PyCapsule with a borrowed pointer to the raw SVN reporter, or - /// None if this reporter is not backed by one. - fn _raw_reporter_capsule<'py>( - &mut self, - py: Python<'py>, - ) -> PyResult>> { - let parts = match self.reporter.as_ref() { - Some(r) => r.as_raw_reporter(), - None => None, - }; - let Some((ptr, baton)) = parts else { - return Ok(None); - }; - // Box the pair and hand the capsule a borrowed pointer to it. The - // pointers borrow from `self.reporter`, which the caller must keep - // alive; we leak the small box (freed when the reporter is dropped via - // _raw_reporter_box below) to avoid Send requirements on PyCapsule::new. - let boxed: Box<(*const std::ffi::c_void, *mut std::ffi::c_void)> = Box::new((ptr, baton)); - let raw = Box::into_raw(boxed); - self._raw_reporter_box = Some(raw); - let non_null = std::ptr::NonNull::new(raw as *mut std::ffi::c_void).unwrap(); - let capsule = unsafe { - pyo3::types::PyCapsule::new_with_pointer(py, non_null, RAW_REPORTER_CAPSULE_NAME)? - }; - Ok(Some(capsule)) - } - /// Set a path #[pyo3(signature = (path, revision, start_empty, lock_token=None, depth=None))] fn set_path( diff --git a/ra/src/session.rs b/ra/src/session.rs index a54fb87f..ec47783f 100644 --- a/ra/src/session.rs +++ b/ra/src/session.rs @@ -1142,53 +1142,18 @@ impl RemoteAccess { let rev = subvertpy_util::to_revnum_or_head(revision); let switch_target = subvertpy_util::to_relpath(switch_target)?; - let depth = if recurse { - subversion::Depth::Infinity - } else { - subversion::Depth::Files - }; - - // If the editor is itself a subvertpy editor (e.g. from - // wc.Context.get_switch_editor), drive its real delta editor directly - // through a capsule rather than bouncing every callback back through - // Python, which would re-enter the interpreter recursively. - let editor_capsule = crate::py_editor::wrap_editor_capsule(slf.py(), &switch_editor); - - if let Some(editor_ref) = editor_capsule { - let reporter = unsafe { - let session_ptr = - &mut slf.borrow_mut().session as *mut subversion::ra::Session<'static>; - let raw_reporter = (*session_ptr) - .do_switch( - rev, - switch_target.as_str(), - depth, - switch_url, - send_copyfrom_args, - ignore_ancestry, - editor_ref, - ) - .map_err(|e| svn_err_to_py(e))?; - std::mem::transmute::< - Box, - Box, - >(raw_reporter) - }; - // Keep the Python editor alive for as long as the reporter, so the - // capsule pointer stays valid. - return Ok(crate::reporter::Reporter::new_with_session_and_keepalive( - reporter, - slf.unbind().into_any(), - switch_editor, - )); - } - let py_editor = crate::py_editor::PyEditorWrapper::new(switch_editor); let wrap_editor = py_editor.into_wrap_editor(); let boxed_editor = Box::new(wrap_editor); let editor_ptr: *mut subversion::delta::WrapEditor = Box::into_raw(boxed_editor); + let depth = if recurse { + subversion::Depth::Infinity + } else { + subversion::Depth::Files + }; + let reporter = unsafe { let editor_ref = &mut *editor_ptr; diff --git a/subvertpy_util/src/editor.rs b/subvertpy_util/src/editor.rs index 73850791..ae57af14 100644 --- a/subvertpy_util/src/editor.rs +++ b/subvertpy_util/src/editor.rs @@ -70,24 +70,6 @@ impl PyEditor { #[pymethods] impl PyEditor { - /// Return a PyCapsule wrapping a borrowed pointer to the underlying - /// `WrapEditor`. - /// - /// This lets the ``ra`` extension module recover the real delta editor when - /// a subvertpy editor (e.g. from ``wc.Context.get_switch_editor``) is handed - /// to ``do_switch`` / ``do_update``, instead of bouncing every callback back - /// through Python. The capsule borrows from this object, so the caller must - /// keep this ``Editor`` alive while using the capsule. - fn _wrap_editor_capsule<'py>( - &self, - py: Python<'py>, - ) -> PyResult> { - let ptr = &self.editor as *const WrapEditor<'static> as *mut std::ffi::c_void; - let non_null = - std::ptr::NonNull::new(ptr).expect("address of a struct field is never null"); - unsafe { pyo3::types::PyCapsule::new_with_pointer(py, non_null, WRAP_EDITOR_CAPSULE_NAME) } - } - fn set_target_revision(&mut self, _py: Python, revision: Option) -> PyResult<()> { let revnum = revision.and_then(|r| to_revnum(r)); self.editor @@ -438,9 +420,6 @@ pub struct PyFileEditor { /// `FileEditor` Python type object (so PyO3 downcasting across modules fails). pub const WRAP_FILE_EDITOR_CAPSULE_NAME: &std::ffi::CStr = c"subvertpy._wrap_file_editor"; -/// PyCapsule name identifying a borrowed `*const WrapEditor<'static>`. -pub const WRAP_EDITOR_CAPSULE_NAME: &std::ffi::CStr = c"subvertpy._wrap_editor"; - #[pymethods] impl PyFileEditor { /// Return a PyCapsule wrapping a borrowed pointer to the underlying diff --git a/wc/src/context.rs b/wc/src/context.rs index 08fbe394..0382321d 100644 --- a/wc/src/context.rs +++ b/wc/src/context.rs @@ -3,78 +3,6 @@ use pyo3::prelude::*; use subvertpy_util::error::svn_err_to_py; -/// Recover a raw SVN reporter (vtable + baton) from a subvertpy reporter -/// object via its capsule. Returns None for any other object. -fn raw_reporter_from_py( - py: Python<'_>, - reporter: &Py, -) -> PyResult> { - let bound = reporter.bind(py); - let capsule = match bound.call_method0("_raw_reporter_capsule") { - Ok(c) => c, - Err(_) => return Ok(None), - }; - if capsule.is_none() { - return Ok(None); - } - let capsule = match capsule.cast::() { - Ok(c) => c, - Err(_) => return Ok(None), - }; - let name = c"subvertpy._raw_reporter"; - let ptr = capsule.pointer_checked(Some(name))?; - // SAFETY: the capsule points to a heap (vtable, baton) pair owned by the - // reporter object, valid while that object is alive. - let pair = - unsafe { *(ptr.as_ptr() as *const (*const std::ffi::c_void, *mut std::ffi::c_void)) }; - Ok(Some(pair)) -} - -/// Adapter exposing a raw SVN reporter to `crawl_revisions5`. Only -/// `as_raw_reporter` is ever called; the trait methods are unreachable here. -struct RawReporter((*const std::ffi::c_void, *mut std::ffi::c_void)); - -impl subversion::ra::Reporter for RawReporter { - fn set_path( - &mut self, - _path: &str, - _rev: subversion::Revnum, - _depth: subversion::Depth, - _start_empty: bool, - _lock_token: &str, - ) -> Result<(), subversion::Error<'static>> { - unreachable!("RawReporter is only used for as_raw_reporter") - } - - fn delete_path(&mut self, _path: &str) -> Result<(), subversion::Error<'static>> { - unreachable!("RawReporter is only used for as_raw_reporter") - } - - fn link_path( - &mut self, - _path: &str, - _url: &str, - _rev: subversion::Revnum, - _depth: subversion::Depth, - _start_empty: bool, - _lock_token: &str, - ) -> Result<(), subversion::Error<'static>> { - unreachable!("RawReporter is only used for as_raw_reporter") - } - - fn finish_report(&mut self) -> Result<(), subversion::Error<'static>> { - unreachable!("RawReporter is only used for as_raw_reporter") - } - - fn abort_report(&mut self) -> Result<(), subversion::Error<'static>> { - unreachable!("RawReporter is only used for as_raw_reporter") - } - - fn as_raw_reporter(&self) -> Option<(*const std::ffi::c_void, *mut std::ffi::c_void)> { - Some(self.0) - } -} - pub(crate) fn depth_to_py(depth: subversion::Depth) -> i32 { match depth { subversion::Depth::Unknown => -2, @@ -520,7 +448,7 @@ impl Context { #[pyo3(signature = (path, reporter, restore_files=true, depth=3, honor_depth_exclude=true, depth_compatibility_trick=false, use_commit_times=false, notify=None))] fn crawl_revisions( &mut self, - py: Python, + _py: Python, path: &Bound, reporter: Py, restore_files: bool, @@ -531,31 +459,11 @@ impl Context { notify: Option>, ) -> PyResult<()> { let path_str = subvertpy_util::py_to_svn_abspath(path)?; - let notify_fn = make_notify_closure(notify); - - // If the reporter is a subvertpy do_update/do_switch reporter, drive the - // crawl against its raw SVN reporter directly. Bouncing each crawl - // callback back through Python (PyReporterBridge) re-enters the - // interpreter recursively and corrupts the in-progress edit. - if let Some(raw) = raw_reporter_from_py(py, &reporter)? { - let mut adapter = RawReporter(raw); - return subversion::wc::crawl_revisions5( - &mut self.inner, - &path_str, - &mut adapter, - restore_files, - depth_from_py(depth), - honor_depth_exclude, - depth_compatibility_trick, - use_commit_times, - notify_fn.as_deref(), - ) - .map_err(svn_err_to_py); - } let py_reporter = PyReporterBridge { reporter }; let mut wrap_reporter = subversion::ra::WrapReporter::from_rust_reporter(py_reporter); + let notify_fn = make_notify_closure(notify); subversion::wc::crawl_revisions5( &mut self.inner, &path_str, From e6c6d8f2a37462e78869d2f4df7f0a68116f608f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Tue, 23 Jun 2026 22:06:02 +0100 Subject: [PATCH 11/13] Deliver txdelta windows from get_file_revs to the Python handler The file-rev callback dropped the handler's returned window callback, so annotate never saw the per-revision deltas. Wire it into a TxDeltaHandler, and treat a negative start revision as 'from the beginning' (revision 0) so the full file history is reported. --- ra/src/session.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/ra/src/session.rs b/ra/src/session.rs index ec47783f..d72b5d21 100644 --- a/ra/src/session.rs +++ b/ra/src/session.rs @@ -867,7 +867,14 @@ impl RemoteAccess { include_merged_revisions: Option, ) -> PyResult<()> { let path = subvertpy_util::to_relpath(path)?; - let start_rev = subvertpy_util::to_revnum_or_head(start); + // A negative start means "from the beginning of the file's history"; + // svn_ra_get_file_revs2 expects revision 0 for that, not an invalid + // revnum (which it would treat as the youngest revision). + let start_rev = if start < 0 { + subversion::Revnum::from(0u64) + } else { + subvertpy_util::to_revnum_or_head(start) + }; let end_rev = subvertpy_util::to_revnum_or_head(end); let py_handler = handler.clone(); @@ -899,11 +906,58 @@ impl RemoteAccess { subversion::Error::from_message(&format!("Failed to convert args: {}", e)) })?; - py_handler.call1(&args).map_err(|e| { + let result = py_handler.call1(&args).map_err(|e| { subversion::Error::from_message(&format!("Python callback error: {}", e)) })?; - Ok(None) + // The Python handler may return a txdelta window callback; if + // so, wire it up so the deltas for this revision are delivered. + if result.is_none() { + return Ok(None); + } + let window_handler = result.unbind(); + let handler: subversion::ra::TxDeltaHandler = Box::new( + move |window: Option<&subversion::delta::TxDeltaWindowRef<'_>>| { + Python::attach(|py| { + let py_window: Py = match window { + None => py.None(), + Some(w) => { + let ops = w.ops(); + let py_ops = pyo3::types::PyList::new( + py, + ops.iter().map(|&(a, o, l)| (a, o, l)), + ) + .map_err(|e| { + subversion::Error::from_message(&format!("{}", e)) + })?; + let new_data = PyBytes::new(py, w.new_data()); + ( + w.sview_offset(), + w.sview_len(), + w.tview_len(), + w.src_ops(), + py_ops, + new_data, + ) + .into_pyobject(py) + .map_err(|e| { + subversion::Error::from_message(&format!("{}", e)) + })? + .into_any() + .unbind() + } + }; + window_handler.call1(py, (py_window,)).map_err(|e| { + subversion::Error::from_message(&format!( + "txdelta window handler error: {}", + e + )) + })?; + Ok(()) + }) + }, + ); + Ok(Some(handler)) }) }; From 582964e83d6a3616c510c869606a42772fb792e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Tue, 23 Jun 2026 22:10:10 +0100 Subject: [PATCH 12/13] Wrap client.revert --- client/src/context.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/client/src/context.rs b/client/src/context.rs index 9bbf6be2..d5451a0b 100644 --- a/client/src/context.rs +++ b/client/src/context.rs @@ -310,6 +310,32 @@ impl Client { Ok(()) } + /// Restore pristine working copy file (undo all local edits) + #[pyo3(signature = (paths, depth=None, recursive=true))] + fn revert( + &mut self, + py: Python, + paths: Vec, + depth: Option>, + recursive: bool, + ) -> PyResult<()> { + let svn_depth = if let Some(d) = depth { + parse_depth(py, &d)? + } else if recursive { + subversion::Depth::Infinity + } else { + subversion::Depth::Empty + }; + let path_refs: Vec<&str> = paths.iter().map(|s| s.as_str()).collect(); + let options = subversion::client::RevertOptions { + depth: svn_depth, + ..Default::default() + }; + self.ctx + .revert(&path_refs, &options) + .map_err(|e| subvertpy_util::error::svn_err_to_py(e)) + } + /// Commit changes to the repository #[pyo3(signature = (targets, recurse=true, keep_locks=true, keep_changelist=false, commit_as_operations=false, include_file_externals=false, include_dir_externals=false, revprops=None, callback=None))] fn commit( From e999b47cb93627657616befc66cfe57262f28d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Wed, 24 Jun 2026 00:06:53 +0100 Subject: [PATCH 13/13] Drop local subversion-rs patch override --- Cargo.lock | 8 -------- Cargo.toml | 6 ------ 2 files changed, 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 96e492e0..168744e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -617,11 +617,3 @@ name = "winnow" version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0592e1c9d151f854e6fd382574c3a0855250e1d9b2f99d9281c6e6391af352f1" - -[[patch.unused]] -name = "subversion" -version = "0.1.11" - -[[patch.unused]] -name = "subversion-sys" -version = "0.1.2" diff --git a/Cargo.toml b/Cargo.toml index 27bfa125..d2ce7936 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,9 +12,3 @@ pyo3 = { version = "0.29" } subversion = { version = "0.1.12" } #subversion = { path = "../subversion-rs" } pyo3-filelike = { version = "0.5" } - -# Use the local subversion-rs checkout that adds Context write-lock support. -# Drop this once those changes are released to crates.io. -[patch.crates-io] -subversion = { path = "../subversion-rs" } -subversion-sys = { path = "../subversion-rs/subversion-sys" }