fix(shares): let add and remove reach a running daemon
shares add wrote the config file and stopped there, so with a daemon running the folder stayed invisible to the network until the daemon restarted — the command reported success and nothing had happened. Locally that is the right meaning, because there is no session yet; a daemon is already the next run. Both commands now push the new list over shares.set. The e2e test was checked against a disabled push to confirm it detects the bug.
This commit is contained in:
@@ -143,6 +143,11 @@ configuration and a client cannot name one — that would be an arbitrary write
|
||||
on the daemon's host. `daemon.status` reports the directory in use. There is
|
||||
no method to fetch the bytes over the control socket.
|
||||
|
||||
**Configuration is per machine, shares are per session.** A client's config
|
||||
file is its own; changing it does not reach a daemon. The exception is
|
||||
`shares.set`, which the CLI's `shares add`/`remove` call so a folder is served
|
||||
without waiting for a restart.
|
||||
|
||||
**Several clients, no locking.** Last write wins. Two clients cancelling the
|
||||
same transfer is two hands on one keyboard, not an error.
|
||||
|
||||
|
||||
@@ -120,7 +120,8 @@ clients never need it.
|
||||
|
||||
**`shares list`** — one object per configured folder: `directory`, `usable`.
|
||||
**`shares add <directory>`** and **`shares remove <directory>`** — change the
|
||||
config file; they print nothing.
|
||||
config file; they print nothing. With a daemon running they also update it in
|
||||
place, so the folder is on the network at once rather than after a restart.
|
||||
**`shares status`** and **`shares reindex`** — `folders`, `files`,
|
||||
`directories`, counted by logging in and scanning.
|
||||
|
||||
|
||||
@@ -5,11 +5,13 @@
|
||||
//! what the next run — interactive or scripted — starts from.
|
||||
|
||||
use super::{Ctx, Session};
|
||||
use crate::api::SessionApi;
|
||||
use crate::directories;
|
||||
use crate::output::{
|
||||
CliError, CliResult, ConfigRecord, Out, ShareRecord, ShareStatusRecord,
|
||||
};
|
||||
use crate::persist::config::FileConfig;
|
||||
use crate::remote::{Endpoint, RemoteSession};
|
||||
use std::path::PathBuf;
|
||||
|
||||
/// Where a `shares`/`config` change is written, and what it starts from.
|
||||
@@ -46,7 +48,42 @@ pub fn shares_list(out: &Out, store: &Store, resolved: &[String]) -> CliResult {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn shares_add(out: &Out, store: &Store, directory: &str) -> CliResult {
|
||||
/// Where a running daemon can be reached, so a share change takes effect now
|
||||
/// rather than at its next restart.
|
||||
pub struct LiveDaemon {
|
||||
pub endpoint: Option<Endpoint>,
|
||||
pub token: Option<String>,
|
||||
}
|
||||
|
||||
/// Tell a running daemon what this machine now shares.
|
||||
///
|
||||
/// Locally, `shares add` means "shared from the next run on", because there is
|
||||
/// no session yet. A daemon *is* that next run and it is already going, so
|
||||
/// leaving it on its startup list would make the command look like it had
|
||||
/// done nothing — the folder configured, and still invisible to the network.
|
||||
fn push_to_daemon(out: &Out, daemon: &LiveDaemon, directories: Vec<String>) {
|
||||
let Some(endpoint) = daemon.endpoint.as_ref() else {
|
||||
return;
|
||||
};
|
||||
let token = daemon.token.clone().or_else(crate::daemon::stored_token);
|
||||
let outcome = RemoteSession::connect(endpoint, token)
|
||||
.and_then(|session| session.set_shared_directories(directories));
|
||||
match outcome {
|
||||
Ok(()) => out.status("the running daemon is re-indexing"),
|
||||
// Not fatal: the config file is written either way, so the change
|
||||
// still takes effect when the daemon next starts.
|
||||
Err(e) => out.warn(&format!(
|
||||
"the daemon kept its own shares ({e}); restart it to pick this up"
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn shares_add(
|
||||
out: &Out,
|
||||
store: &Store,
|
||||
directory: &str,
|
||||
daemon: &LiveDaemon,
|
||||
) -> CliResult {
|
||||
// Refuse a folder that is not there: sharing it would silently index
|
||||
// nothing, and the mistake would only surface as "no one can find my
|
||||
// files" much later.
|
||||
@@ -63,14 +100,20 @@ pub fn shares_add(out: &Out, store: &Store, directory: &str) -> CliResult {
|
||||
return Ok(());
|
||||
}
|
||||
dirs.push(directory.to_string());
|
||||
updated.shared_dirs = Some(dirs);
|
||||
updated.shared_dirs = Some(dirs.clone());
|
||||
updated.shared_dir = None;
|
||||
store.save(&updated)?;
|
||||
out.status(&format!("sharing {directory}"));
|
||||
push_to_daemon(out, daemon, dirs);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn shares_remove(out: &Out, store: &Store, directory: &str) -> CliResult {
|
||||
pub fn shares_remove(
|
||||
out: &Out,
|
||||
store: &Store,
|
||||
directory: &str,
|
||||
daemon: &LiveDaemon,
|
||||
) -> CliResult {
|
||||
let mut dirs = share_list(&store.config);
|
||||
let before = dirs.len();
|
||||
dirs.retain(|existing| existing != directory);
|
||||
@@ -81,10 +124,11 @@ pub fn shares_remove(out: &Out, store: &Store, directory: &str) -> CliResult {
|
||||
}
|
||||
|
||||
let mut updated = store.config.clone();
|
||||
updated.shared_dirs = Some(dirs);
|
||||
updated.shared_dirs = Some(dirs.clone());
|
||||
updated.shared_dir = None;
|
||||
store.save(&updated)?;
|
||||
out.status(&format!("no longer sharing {directory}"));
|
||||
push_to_daemon(out, daemon, dirs);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
+16
-2
@@ -84,12 +84,18 @@ pub fn run(mut cli: Cli, out: &Out) -> CliResult {
|
||||
}
|
||||
Commands::Shares(SharesCommand::Add { ref directory }) => {
|
||||
return crate::commands::settings::shares_add(
|
||||
out, &store, directory,
|
||||
out,
|
||||
&store,
|
||||
directory,
|
||||
&live_daemon(&cli),
|
||||
);
|
||||
}
|
||||
Commands::Shares(SharesCommand::Remove { ref directory }) => {
|
||||
return crate::commands::settings::shares_remove(
|
||||
out, &store, directory,
|
||||
out,
|
||||
&store,
|
||||
directory,
|
||||
&live_daemon(&cli),
|
||||
);
|
||||
}
|
||||
Commands::Wish(WishCommand::Add { ref query }) => {
|
||||
@@ -243,6 +249,14 @@ fn context(
|
||||
})
|
||||
}
|
||||
|
||||
/// The daemon a config change should also reach, if one is running.
|
||||
fn live_daemon(cli: &Cli) -> crate::commands::settings::LiveDaemon {
|
||||
crate::commands::settings::LiveDaemon {
|
||||
endpoint: daemon_endpoint(cli),
|
||||
token: cli.daemon_token.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Ask a daemon what it is, so this run describes the session it will actually
|
||||
/// use. A daemon that cannot be reached is left to `Session::open` to report,
|
||||
/// which produces a better message than anything this early probe could.
|
||||
|
||||
@@ -3395,3 +3395,70 @@ fn the_daemon_token_is_stable_across_reads() {
|
||||
"a restart must not lock existing clients out"
|
||||
);
|
||||
}
|
||||
|
||||
/// Adding a folder reaches a daemon that is already running.
|
||||
///
|
||||
/// Locally `shares add` means "shared from the next run on", because there is
|
||||
/// no session yet. A daemon *is* that next run and it is already going, so
|
||||
/// without pushing the change the command would look like it had worked while
|
||||
/// the network still saw nothing.
|
||||
#[test]
|
||||
fn adding_a_share_takes_effect_on_a_running_daemon() {
|
||||
if mode() != Mode::Daemon {
|
||||
println!(
|
||||
"skipped: there is no running session to update in local mode"
|
||||
);
|
||||
return;
|
||||
}
|
||||
let server = server_or_skip!();
|
||||
let user = "cli_e2e_live_shares";
|
||||
let socket = server.daemon_state(user).join("daemon.sock");
|
||||
let at = socket.display().to_string();
|
||||
let config = Scratch::new("live-shares-config");
|
||||
let config_file = config.path().join("config.toml");
|
||||
let share = Scratch::new("live-shares");
|
||||
std::fs::write(share.path().join("cli_probe_live.bin"), probe_bytes())
|
||||
.expect("share file");
|
||||
|
||||
let before =
|
||||
run(&["--no-config", "--json", "--daemon", &at, "shares", "status"]);
|
||||
assert_eq!(code(&before), EXIT_OK, "stderr: {}", stderr(&before));
|
||||
|
||||
let added = run(&[
|
||||
"--config",
|
||||
config_file.to_str().expect("utf-8 path"),
|
||||
"--daemon",
|
||||
&at,
|
||||
"shares",
|
||||
"add",
|
||||
&share.display(),
|
||||
]);
|
||||
assert_eq!(code(&added), EXIT_OK, "stderr: {}", stderr(&added));
|
||||
|
||||
// The daemon re-indexes on the way through, so the folder is visible to
|
||||
// the network now rather than after a restart.
|
||||
let after: serde_json::Value = serde_json::from_str(
|
||||
records(&run(&[
|
||||
"--no-config",
|
||||
"--json",
|
||||
"--daemon",
|
||||
&at,
|
||||
"shares",
|
||||
"status",
|
||||
]))
|
||||
.first()
|
||||
.expect("one record"),
|
||||
)
|
||||
.expect("valid JSON");
|
||||
let listed = after["directories"]
|
||||
.as_array()
|
||||
.expect("directories is an array");
|
||||
assert!(
|
||||
listed.iter().any(|d| d == &share.display()),
|
||||
"the daemon should be sharing the folder that was just added: {after}"
|
||||
);
|
||||
assert!(
|
||||
after["files"].as_u64().is_some_and(|files| files >= 1),
|
||||
"and should have indexed what is in it: {after}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user