impl WebDavCmd {
// [+] &self is a reference to a `WebDavCmd` where the fields are only representing the CLI args
// we musn't use it because it lacks the config from `ENV` or `TOML`
fn inner_run(&self) -> Result<()> {
// [+] this config is complete, the TOML, ENV and CLI were merged to produce this config struct instance.
// the correct `WebDavCmd` can be found in `RUSTIC_APP.config().webdav`
let config = RUSTIC_APP.config();
let repo = open_repository_indexed(&config.repository)?;
// [+] this is the bugged code.
// Here by using `self` we ignore the `path_template` that could be defined in the `rustic.toml`
let path_template = self
.path_template
.clone()
.unwrap_or_else(|| "[{hostname}]/[{label}]/{time}".to_string());
// [+] same here
let time_template = self
.time_template
.clone()
.unwrap_or_else(|| "%Y-%m-%d_%H-%M-%S".to_string());
// ...
// [+] same here
let addr = self
.address
.clone()
.unwrap_or_else(|| "localhost:8000".to_string())
.to_socket_addrs()?
.next()
.ok_or_else(|| anyhow!("no address given"))?;
// ....
To correct this, the #1241 PR came with changes similar to:
- let path_template = self
+ let path_template = config
+ .webdav
.path_template
.clone()
.unwrap_or_else(|| "[{hostname}]/[{label}]/{time}".to_string());
More complex problem
The forget command has CLI-only options that are only accessible from its inner_run&self reference.
But the same issue exists with this command, the other options are merged from the TOML and the ENV.
This bug is purely a developers' problem. By the current design, the type system is not able to verify that we don't use self when we shouldn't. And sometimes we should (forget command for instance).
For this reason, we need to reflect on design updates and refactors that would solve this issue.
The design reflection will include (but not only) the role of the abscissa framework, our usage of it, and how it fits to our needs.
This issue will track the related design change.
Do not hesitate if you have any prompt, question, advice and all.
Context and description
rustic
has three complementary ways of being prompted a configuration.the current design of rustic makes it that the implementation of command (such as
forget
orwebdav
) has access to two different configurations:RUSTIC_APP.config()
where all the sources were merged&self
reference to the CLI (clap parsed) instance that only has the CLIExample
issue: #1163 fix PR: #1241
code before the fix (https://github.com/rustic-rs/rustic/blob/abf1835cbdf5804806a1106cfb88b4cd3b20e0d9/src/commands/webdav.rs#L68C1-L104C1):
To correct this, the #1241 PR came with changes similar to:
More complex problem
The
forget
command has CLI-only options that are only accessible from itsinner_run
&self
reference. But the same issue exists with this command, the other options are merged from the TOML and the ENV.See https://github.com/rustic-rs/rustic/blob/abf1835cbdf5804806a1106cfb88b4cd3b20e0d9/src/commands/forget.rs#L100-L111
Why an issue, if this is fixed
This bug is purely a developers' problem. By the current design, the type system is not able to verify that we don't use
self
when we shouldn't. And sometimes we should (forget
command for instance).For this reason, we need to reflect on design updates and refactors that would solve this issue. The design reflection will include (but not only) the role of the
abscissa
framework, our usage of it, and how it fits to our needs.This issue will track the related design change.
Do not hesitate if you have any prompt, question, advice and all.