Skip to content

Commit

Permalink
Write about user interfaces and the use/non-use of async
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Aug 6, 2020
1 parent bca7ce2 commit 91ba045
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 26 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ small = ["lean-cli"]
fast = ["git-features/parallel", "git-features/fast-sha1"]
pretty-cli = ["structopt",
"git-features/progress-prodash",
"git-features/interuptible",
"git-features/interruptible",
"gitoxide-core/serde1",
"prodash/log-renderer",
"prodash-tui-renderer",
"prodash-line-renderer",
"prodash/localtime",
"env_logger",
"futures-lite"]
lean-cli = ["argh", "git-features/progress-log", "env_logger", "git-features/interuptible"]
lean-cli = ["argh", "git-features/progress-log", "env_logger", "git-features/interruptible"]

prodash-line-renderer-crossterm = ["prodash-line-renderer", "prodash/line-renderer-crossterm", "git-features/progress-prodash", "atty", "crosstermion"]
prodash-line-renderer-termion = ["prodash-line-renderer", "prodash/line-renderer-termion", "git-features/progress-prodash", "atty", "crosstermion"]
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ check: ## Build all code in suitable configurations
cd git-features && cargo check --all-features \
&& cargo check --features parallel \
&& cargo check --features fast-sha1 \
&& cargo check --features interuptible
&& cargo check --features interruptible

unit-tests: ## run all unit tests
cargo test --all --no-fail-fast
Expand Down
36 changes: 29 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,6 @@ Provide a CLI to for the most basic user journey:
* [ ] create (thin) pack
* [ ] transform _all_ `unwrap()` calls into `expect(…)` or `?` for quality. `parking_lot` will help for unwraps of poisoned locks.

### Roadmap to 1.1

* [ ] clone a repository via
* [ ] ssh

## Cargo features guide

Cargo uses feature toggles to control which dependencies are pulled in, allowing users to specialize crates to fit their usage.
Expand Down Expand Up @@ -280,7 +275,7 @@ There are **convenience features**, which combine common choices of the above in
* **light** = *lean-cli* + *fast*
* crossplatform by nature as this comes with simplified log based progress
* **small** = *lean-cli*
* As small as it can possibly be, no threading, no fast sha1, log based progress only
* As small as it can possibly be, no threading, no fast sha1, log based progress only, no cleanup of temporary files on interrupt

### git-features

Expand All @@ -301,7 +296,7 @@ All feature toggles are additive.
* **progress-prodash**
* Implement the `Progress` trait for the tree data structures provided by `prodash`, which enables using a terminal user
interface for progress.
* **interuptible**
* **interruptible**
* Listen to interrupts and termination requests and provide long-running operations tooling to allow aborting the input stream.
* If unset, these utilities will be a no-op which may lead to leaking temporary files when interrupted.
* If the application already sets a handler, this handler will have no effect.
Expand Down Expand Up @@ -342,6 +337,32 @@ All feature toggles are additive.

### Guidelines

* **async**
* **library client-side**
* Don't use it client side, as operations there are usually bound by the CPU and ultra-fast access to memory mapped files.
It's no problem to saturate either CPU or the IO system.
* **User Interfaces**
* User interfaces can greatly benefit from using async as it's much easier to maintain a responsive UI thread that way thanks
to the wonderful future combinators.
* `blocking` can be used to make `Read` and `Iterator` async, or move any operation onto a thread which blends it into the
async world.
* Most operations are fast and 'interrupting' them is as easy as ignoring their result by cancelling their task.
* Long-running operations can be roughly interacted with using `git_features::interruptible::interrupt()` function, and after a moment
of waiting the flag can be unset with the `…::uninterrupt()` function to allow new long-running operations to work.
Every long running operation supports this.
* **server-side**
* Building a pack is CPU and at some point, IO bound, and it makes no sense to use async to handle more connections - git
needs a lot of resources and threads will do just fine.

* **interruption of long-running operations**
* Use `git-features::interruptible::*` for building support for interruptions of long-running operations only.
* It's up to the author to decide how to best integrate it, generally we use a poll-based mechanism to check whether
an interrupt flag is set.
* **this is a must if…**
* …temporary resources like files might otherwise be leaked
* **this is optional but desirable if…**
* …there is no leakage otherwise to support user interfaces. They background long-running operations and need them to be cancellable.

* **prepare for SHA256 support by using `owned::Id` and `borrowed::Id`**
* eventually there will be the need to support both Sha1 and Sha256. We anticipate it by using the `Id` type instead
of slices or arrays of 20 bytes. This way, eventually we can support multiple hash digest sizes.
Expand Down Expand Up @@ -381,6 +402,7 @@ From there, we can derive a few rules to try adhere to:

* does not show any progress or logging output by default
* if supported and logging is enabled, it will show timestamps in UTC
* it does not need a git repository, but instead takes all variables via the command-line

### Porcelain

Expand Down
4 changes: 2 additions & 2 deletions git-features/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ parallel = ["crossbeam-utils", "crossbeam-channel", "num_cpus"]
fast-sha1 = ["fastsha1"]
progress-log = ["log"]
progress-prodash = ["prodash"]
interuptible = ["ctrlc", "once_cell"]
interruptible = ["ctrlc", "once_cell"]

[[test]]
name = "parallel"
Expand Down Expand Up @@ -48,7 +48,7 @@ fastsha1 = { package = "sha-1", version = "0.9.1", optional = true }
log = { version = "0.4.8", optional = true }
prodash = { version = "7.0.2", optional = true, default-features = false }

# interuptible
# interruptible
ctrlc = { version = "3.1.4", optional = true, default-features = false, features = ['termination'] }
once_cell = { version = "1.4.0", optional = true, default-features = false, features = ["std"] }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(feature = "interuptible")]
#[cfg(feature = "interruptible")]
mod _impl {
use ctrlc;
use once_cell::sync::Lazy;
Expand All @@ -24,23 +24,26 @@ mod _impl {
R: io::Read,
{
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
if is_interupted() {
if is_interrupted() {
return Err(io::Error::new(io::ErrorKind::Other, "interrupted by user"));
}
self.inner.read(buf)
}
}

pub fn is_interupted() -> bool {
pub fn is_interrupted() -> bool {
IS_INTERRUPTED.load(Ordering::Relaxed)
}

pub fn interupt() {
pub fn interrupt() {
IS_INTERRUPTED.store(true, Ordering::Relaxed);
}
pub fn uninterrupt() {
IS_INTERRUPTED.store(false, Ordering::Relaxed);
}
}

#[cfg(not(feature = "interuptible"))]
#[cfg(not(feature = "interruptible"))]
mod _impl {
use std::io;

Expand All @@ -60,6 +63,10 @@ mod _impl {
pub fn is_interrupted() -> bool {
false
}

pub fn interrupt() {}

pub fn uninterrupt() {}
}

pub use _impl::*;
2 changes: 1 addition & 1 deletion git-features/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![forbid(unsafe_code)]

pub mod hash;
pub mod interuptible;
pub mod interruptible;
pub mod parallel;
pub mod progress;
4 changes: 2 additions & 2 deletions git-odb/src/pack/bundle/write/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::pack;
use git_features::{interuptible, progress, progress::Progress};
use git_features::{interruptible, progress, progress::Progress};
use std::{
io,
path::{Path, PathBuf},
Expand Down Expand Up @@ -53,7 +53,7 @@ impl pack::Bundle {
};
let data_path: Option<PathBuf> = data_file.as_ref().map(|f| f.as_ref().into());
let mut pack = PassThrough {
reader: interuptible::Read { inner: pack },
reader: interruptible::Read { inner: pack },
writer: data_file,
};
let eight_pages = 4096 * 8;
Expand Down
8 changes: 7 additions & 1 deletion git-odb/src/pack/tree/traverse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
pack::data::EntrySlice,
pack::tree::{Item, Tree},
};
use git_features::{parallel, parallel::in_parallel_if, progress::Progress};
use git_features::{interruptible::is_interrupted, parallel, parallel::in_parallel_if, progress::Progress};
use quick_error::quick_error;

mod resolve;
Expand All @@ -23,6 +23,9 @@ quick_error! {
source(&**err)
from()
}
Interrupted {
display("Interrupted by user")
}
}
}

Expand Down Expand Up @@ -112,6 +115,9 @@ where
let input = input?;
self.item_count += input;
self.progress.lock().set(self.item_count as u32);
if is_interrupted() {
return Err(Error::Interrupted);
}
Ok(())
}

Expand Down
6 changes: 3 additions & 3 deletions src/plumbing/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ fn prepare_and_run<T: Send + 'static>(
+ 'static,
) -> Result<T> {
super::init_env_logger(false);
use git_features::interuptible::{interupt, is_interupted};
use git_features::interruptible::{interrupt, is_interrupted};
match (verbose, progress) {
(false, false) => run(None, &mut stdout(), &mut stderr()),
(true, false) => {
Expand All @@ -214,7 +214,7 @@ fn prepare_and_run<T: Send + 'static>(
let tx = tx.clone();
move || loop {
std::thread::sleep(std::time::Duration::from_millis(500));
if is_interupted() {
if is_interrupted() {
tx.send(Event::UIDone).ok();
break;
}
Expand Down Expand Up @@ -274,7 +274,7 @@ fn prepare_and_run<T: Send + 'static>(
Event::UIDone => {
// We don't know why the UI is done, usually it's the user aborting.
// We need the computation to stop as well so let's wait for that to happen
interupt();
interrupt();
continue;
}
Event::ComputationDone(res, out, err) => {
Expand Down
4 changes: 2 additions & 2 deletions tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
system temp files)
* [x] for lean mode
* [x] for pretty mode
* [ ] allow interrupting the resolution phase too
* [ ] fix typo :D - thanks IJ for confusing me
* [x] allow interrupting the resolution phase too
* [x] fix typo :D - thanks IJ for confusing me
* [ ] move --verbose, --progress and --progress-keep-open to the top-level
* [ ] unit tests for bundle index write
* [ ] journey test for command-line capabilities
Expand Down

0 comments on commit 91ba045

Please sign in to comment.