From 0d5f6e4ba6bf00837f6546ffe3f12fd31250e4a3 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 6 Jan 2025 13:07:42 -0500 Subject: [PATCH] Update to Taskchampion-2.0.1 (#37) Co-authored-by: illya-laifu <110652325+illya-laifu@users.noreply.github.com> --- .github/workflows/checks.yml | 2 +- .github/workflows/ci.yml | 1 - .github/workflows/tests.yml | 4 +--- Cargo.lock | 6 +++--- Cargo.toml | 6 +++--- pyproject.toml | 2 +- src/access_mode.rs | 27 +++++++++++++++++++++++++++ src/dependency_map.rs | 15 +++++---------- src/lib.rs | 3 +++ src/replica.rs | 26 +++++++++++--------------- src/task/annotation.rs | 2 +- src/task/task.rs | 13 ++----------- src/working_set.rs | 14 +------------- taskchampion.pyi | 11 ++++++++++- tests/test_replica.py | 11 +++++++++-- 15 files changed, 78 insertions(+), 65 deletions(-) create mode 100644 src/access_mode.rs diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index b66c32a..67026e8 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -29,7 +29,7 @@ jobs: - uses: actions-rs/toolchain@v1 with: - toolchain: "1.78.0" # MSRV + toolchain: "1.81.0" # MSRV override: true components: clippy diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6217470..0cc5097 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -191,7 +191,6 @@ jobs: - uses: actions/setup-python@v5 with: python-version: 3.9 - # See #36 for upgrading to poetry 2.0. - run: pip install 'poetry<2' - run: poetry install - run: poetry run pip install wheels-linux-x86_64/* diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 88d914c..ea10f2d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -29,15 +29,13 @@ jobs: - uses: actions-rs/toolchain@v1 with: - toolchain: "1.78.0" # MSRV + toolchain: "1.81.0" # MSRV override: true components: clippy - uses: actions/setup-python@v5 with: python-version: 3.9 - - # See #36 for upgrading to poetry 2.0. - run: pip install 'poetry<2' - run: poetry install - run: poetry run maturin develop diff --git a/Cargo.lock b/Cargo.lock index 49e9768..b80f565 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2362,9 +2362,9 @@ checksum = "61c41af27dd6d1e27b1b16b489db798443478cef1f06a660c96db617ba5de3b1" [[package]] name = "taskchampion" -version = "1.0.2" +version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c4e41712ec2dd9cb5e7f4f20daf13a8e0937a927fb886153f2523a6686c35983" +checksum = "5ba26a986269f2b24f34d64f777abca9abb3f2923fd83111ce831100f292459f" dependencies = [ "anyhow", "aws-config", @@ -2390,7 +2390,7 @@ dependencies = [ [[package]] name = "taskchampion-py" -version = "1.0.2" +version = "2.0.1" dependencies = [ "chrono", "pyo3", diff --git a/Cargo.toml b/Cargo.toml index fdf2e92..a421624 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,9 +1,9 @@ [package] name = "taskchampion-py" -version = "1.0.2" +version = "2.0.1" edition = "2021" # This should match the MSRV of the `taskchampion` crate. -rust-version = "1.78.0" +rust-version = "1.81.0" [package.metadata.maturin] name = "taskchampion" @@ -16,4 +16,4 @@ doc = false [dependencies] pyo3 = { version = "0.22.6", features = ["chrono"] } chrono = "*" -taskchampion = { version = "=1.0.2" } +taskchampion = { version = "=2.0.1" } diff --git a/pyproject.toml b/pyproject.toml index a872202..854312b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "maturin" [project] name = "taskchampion-py" -version = "1.0.2" +version = "2.0.1" requires-python = ">=3.9" classifiers = [ "Programming Language :: Rust", diff --git a/src/access_mode.rs b/src/access_mode.rs new file mode 100644 index 0000000..07b9eb5 --- /dev/null +++ b/src/access_mode.rs @@ -0,0 +1,27 @@ +use pyo3::prelude::*; +pub use taskchampion::storage::AccessMode as TCAccessMode; + +#[pyclass(eq, eq_int)] +#[derive(Clone, Copy, PartialEq)] +pub enum AccessMode { + ReadOnly, + ReadWrite, +} + +impl From for AccessMode { + fn from(status: TCAccessMode) -> Self { + match status { + TCAccessMode::ReadOnly => AccessMode::ReadOnly, + TCAccessMode::ReadWrite => AccessMode::ReadWrite, + } + } +} + +impl From for TCAccessMode { + fn from(status: AccessMode) -> Self { + match status { + AccessMode::ReadOnly => TCAccessMode::ReadOnly, + AccessMode::ReadWrite => TCAccessMode::ReadWrite, + } + } +} diff --git a/src/dependency_map.rs b/src/dependency_map.rs index e1ff65c..435d451 100644 --- a/src/dependency_map.rs +++ b/src/dependency_map.rs @@ -1,14 +1,11 @@ use pyo3::prelude::*; +use std::sync::Arc; use taskchampion::{DependencyMap as TCDependencyMap, Uuid}; // See `Replica::dependency_map` for the rationale for using a raw pointer here. #[pyclass] -pub struct DependencyMap(*const TCDependencyMap); - -// SAFETY: `Replica::dependency_map` ensures that the TCDependencyMap is never freed (as the Rc is -// leaked) and TaskChampion does not modify it, so no races can occur. -unsafe impl Send for DependencyMap {} +pub struct DependencyMap(Arc); #[pymethods] impl DependencyMap { @@ -33,16 +30,14 @@ impl DependencyMap { } } -impl From<*const TCDependencyMap> for DependencyMap { - fn from(value: *const TCDependencyMap) -> Self { +impl From> for DependencyMap { + fn from(value: Arc) -> Self { DependencyMap(value) } } impl AsRef for DependencyMap { fn as_ref(&self) -> &TCDependencyMap { - // SAFETY: `Replica::dependency_map` ensures that the TCDependencyMap is never freed (as - // the Rc is leaked) and TaskChampion does not modify it, so no races can occur. - unsafe { &*self.0 as &TCDependencyMap } + Arc::as_ref(&self.0) } } diff --git a/src/lib.rs b/src/lib.rs index 8b85854..7ccb6b0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,6 +9,8 @@ pub mod dependency_map; use dependency_map::*; pub mod operation; use operation::*; +pub mod access_mode; +use access_mode::*; pub mod operations; use operations::*; mod task; @@ -28,6 +30,7 @@ fn taskchampion(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_class::()?; m.add_class::()?; m.add_class::()?; + m.add_class::()?; Ok(()) } diff --git a/src/replica.rs b/src/replica.rs index 932b332..18a2ab8 100644 --- a/src/replica.rs +++ b/src/replica.rs @@ -1,9 +1,8 @@ use crate::task::TaskData; use crate::util::{into_runtime_error, uuid2tc}; -use crate::{DependencyMap, Operations, Task, WorkingSet}; +use crate::{AccessMode, DependencyMap, Operations, Task, WorkingSet}; use pyo3::prelude::*; use std::collections::HashMap; -use std::rc::Rc; use taskchampion::{Replica as TCReplica, ServerConfig, StorageConfig}; #[pyclass(unsendable)] @@ -22,13 +21,21 @@ impl Replica { /// Args: /// path (str): path to the directory with the database /// create_if_missing (bool): create the database if it does not exist + /// access_mode (AccessMode): controls whether write access is allowed /// Raises: /// RuntimeError: if database does not exist, and create_if_missing is false - pub fn new_on_disk(path: String, create_if_missing: bool) -> PyResult { + + #[pyo3(signature=(path, create_if_missing, access_mode=AccessMode::ReadWrite))] + pub fn new_on_disk( + path: String, + create_if_missing: bool, + access_mode: AccessMode, + ) -> PyResult { Ok(Replica(TCReplica::new( StorageConfig::OnDisk { taskdb_dir: path.into(), create_if_missing, + access_mode: access_mode.into(), } .into_storage() .map_err(into_runtime_error)?, @@ -91,19 +98,8 @@ impl Replica { } pub fn dependency_map(&mut self, force: bool) -> PyResult { - // `Rc` is not thread-safe, so we must get an owned copy of the data it contains. - // Unfortunately, it cannot be cloned, so this is impossible (but both issues are fixed in - // https://github.com/GothenburgBitFactory/taskchampion/pull/514). - // - // Until that point, we leak the Rc (preventing it from ever being freed) and use a static - // reference to its contents. This is safe based on the weak but currently valid assumption - // that TaskChampion does not modify a DependencyMap after creating it. - // - // This is a temporary hack, and should not be used in "real" code! let dm = self.0.dependency_map(force).map_err(into_runtime_error)?; - // NOTE: this does not decrement the reference count and thus "leaks" the Rc. - let dm_ptr = Rc::into_raw(dm); - Ok(dm_ptr.into()) + Ok(dm.into()) } pub fn get_task(&mut self, uuid: String) -> PyResult> { diff --git a/src/task/annotation.rs b/src/task/annotation.rs index 4899b88..ce3fa20 100644 --- a/src/task/annotation.rs +++ b/src/task/annotation.rs @@ -3,7 +3,7 @@ use pyo3::prelude::*; use taskchampion::Annotation as TCAnnotation; #[pyclass(frozen, eq)] -#[derive(PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] /// An annotation for the task pub struct Annotation(TCAnnotation); diff --git a/src/task/task.rs b/src/task/task.rs index 56e97bd..e9af793 100644 --- a/src/task/task.rs +++ b/src/task/task.rs @@ -5,9 +5,7 @@ use chrono::{DateTime, Utc}; use pyo3::prelude::*; use taskchampion::Task as TCTask; -// TODO: This type can be send once https://github.com/GothenburgBitFactory/taskchampion/pull/514 -// is available. -#[pyclass(unsendable)] +#[pyclass] /// A TaskChampion Task. /// /// This type is not Send, so it cannot be used from any thread but the one where it was created. @@ -264,14 +262,7 @@ impl Task { .map_err(into_runtime_error) } - pub fn add_annotation( - &mut self, - annotation: &Annotation, - ops: &mut Operations, - ) -> PyResult<()> { - // Create an owned annotation (TODO: not needed once - // https://github.com/GothenburgBitFactory/taskchampion/pull/517 is available) - let annotation = Annotation::new(annotation.entry(), annotation.description()); + pub fn add_annotation(&mut self, annotation: Annotation, ops: &mut Operations) -> PyResult<()> { self.0 .add_annotation(annotation.into(), ops.as_mut()) .map_err(into_runtime_error) diff --git a/src/working_set.rs b/src/working_set.rs index ceba6ed..f8f4259 100644 --- a/src/working_set.rs +++ b/src/working_set.rs @@ -1,5 +1,3 @@ -use std::fmt; - use pyo3::prelude::*; use taskchampion::Uuid; use taskchampion::WorkingSet as TCWorkingSet; @@ -29,7 +27,7 @@ impl WorkingSet { } pub fn __repr__(&self) -> String { - format!("{:?}", self) + format!("{:?}", self.0) } pub fn largest_index(&self) -> usize { @@ -61,16 +59,6 @@ impl WorkingSet { } } -// TODO: Use the Taskchampion Debug implementation when -// https://github.com/GothenburgBitFactory/taskchampion/pull/520 is available. -impl fmt::Debug for WorkingSet { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("WorkingSet {")?; - f.debug_list().entries(self.0.iter()).finish()?; - f.write_str("}") - } -} - impl AsRef for WorkingSet { fn as_ref(&self) -> &TCWorkingSet { &self.0 diff --git a/taskchampion.pyi b/taskchampion.pyi index db93659..56b0a06 100644 --- a/taskchampion.pyi +++ b/taskchampion.pyi @@ -18,7 +18,11 @@ __all__ = [ @final class Replica: @staticmethod - def new_on_disk(path: str, create_if_missing: bool): ... + def new_on_disk( + path: str, + create_if_missing: bool, + access_mode: "AccessMode" = AccessMode.ReadWrite, + ): ... @staticmethod def new_in_memory(): ... def create_task(self, uuid: str, ops: "Operations") -> "Task": ... @@ -48,6 +52,11 @@ class Replica: def commit_operations(self, ops: "Operations") -> None: ... def commit_reversed_operations(self, operations: "Operations") -> None: ... +@final +class AccessMode(Enum): + ReadWrite = 1 + ReadOnly = 2 + @final class Operation: @staticmethod diff --git a/tests/test_replica.py b/tests/test_replica.py index 7f1b5f3..1e2e45b 100644 --- a/tests/test_replica.py +++ b/tests/test_replica.py @@ -2,7 +2,7 @@ from pathlib import Path import pytest -from taskchampion import Replica, Operations +from taskchampion import Replica, Operations, AccessMode @pytest.fixture @@ -23,7 +23,6 @@ def replica_with_tasks(empty_replica: Replica): def test_constructor(tmp_path: Path): r = Replica.new_on_disk(str(tmp_path), True) - assert r is not None @@ -47,6 +46,14 @@ def test_constructor_throws_error_with_missing_database(tmp_path: Path): Replica.new_on_disk(str(tmp_path), False) +def test_read_only(tmp_path: Path): + r = Replica.new_on_disk(str(tmp_path), True, AccessMode.ReadOnly) + ops = Operations() + r.create_task(str(uuid.uuid4()), ops) + with pytest.raises(RuntimeError): + r.commit_operations(ops) + + def test_create_task(empty_replica: Replica): u = uuid.uuid4()