--as-path <PATH> option skips empty string

[F3-L1x]

Hello wonderful people,

at first, I'm so grateful for this great project. :3

Just a minor thing.
I noticed, that using the '--as-path' cli-option (when creating a backup) doesn't recognize an empty path, while the configuration-option does.
It would be nice, if both had this ability.

Thank you for your time. (:

[aawsome]

Thanks @F3-L1x for opening this issue!

Can you please write how you did run the CLI command?
Did you try to use --as-path ""?

[F3-L1x]

Wow, what a fast reply.^^
You're welcome. (:

I tried it different ways:

1. With minimal configuration (only specifying host, no-cache, password-file and repository-name) rustic backup --as-path "" ./ I get:

error: no backup source given.

2. Same minimal config as in 1. and command rustic backup --as-path="" ./ gives:

error: a value is required for '--as-path ' but none was supplied

3. With a little more extensive configuration (additionally containing: backup.sources/source = "." and some other stuff) rustic -P local backup --as-path "" ./ returns no errors, but rustic -P local snapshot shows an absolute path, rather than a relative.
4. When additionally adding backup/as-path to the config-file, rustic -P local backup ./ gives me the result, I wished for.
   (No errors and showing an (empty) relative path when using snapshot-cmd)

[aawsome]

First, am I right that your original intend is to run rustic backup ./ and have it saved as a relative path? I think this should not be addressed using --as-path, but instead the path sanitizing should be enhanced fixed such that it handles a leading .. I'll look after that.

Moreover, I think that in fact there shouldn't be a use-case where you need to use an empty --as-path, but use --as-path=. (with a fixed path sanitizing, see above) or -as-path=/ (which already works).

Still, digging a bit into details:
About 1. and 2.: I think that this is an issue of clap parsing the CLI arguments. It seems that clap treats an path "" like a not-given path. This explains why this behaves differently than specifying as-path in the config profile. However, I cannot reproduce the behavior described in 1.

About 3: This is treated by clap like no --as-path would be given which explains this current behavior.

[aawsome]

Correct handling of paths starting with . (including the paths . or ./) should be fixed by rustic-rs/rustic_core#213

[F3-L1x]

[...] your original intend is to run rustic backup ./ and have it saved as a relative path?

Yes that's what I was trying to do.

I think this should not be addressed using --as-path, but instead the path sanitizing should be enhanced fixed such that it handles a leading .. I'll look after that.

You mean, it should be automatically saved as a relative-path, when using ./ as backup source-path?
I thought of it beeing two separate things. 1. Selecting the working dir with ./ and after it choosing to back it up either as the absolute path or relative by using the --as-path - option.
I actually like, that the user can decide, whether it will be saved relative or absolute (even, when using a relative path as a source).
But maybe, I didn't understand you correctly here.

Thanks for you tip with the:

-as-path=/

That did the trick. (:
And as you've mentioned, -as-path='.' / -as-path='./' didn't work properly,
I ended up using an empty string, rather than -as-path=/, as this looked like an absolute path to me (even if when was on Windows).

Thank you for your super quick help and I'll try your fix soon. :) 👍

Edit:

However, I cannot reproduce the behavior described in 1.

It looks like, this has sth. to do with, how powershell handles parameters/arguments (probably discarding empty strings and gobbling the next available argument/option). I get a different output from cmd and msys-bash:

error: a value is required for '--as-path ' but none was supplied

For more information, try '--help'.

[aawsome]

You mean, it should be automatically saved as a relative-path, when using ./ as backup source-path?

Exactly. This is also the current behavior if you backup a relative path without leading .. So it should be consistently the identical thing with a leading . - and that's what's implemented in the mentioned PR.

If you want to backup an absolute path, you can simply specify the full path.

[F3-L1x]

At first - excuse me please, for responding so late (and accidentally closing the issue prematurely).

I've multiple points to add:

tl;dr

I'm (currently) fine with using source (cli-argument or config-value): './' and as-path config-value: '', which don't produce any errors or warnings for me.
So actually - if there's no need to change the inconsistency mentioned in 2.2. and to prevent the warning of 3. - this issue might therefore become closed again from my side. (:

1. introduction

1.1. unclear problem description

I recently read through the discussion and noticed, I probably hadn't made the distinction between the source-path and the paths inside the repository clear enough.
To rectify this: I was actually talking about the repository-paths - so that I may move the repo to another directory and use it there without any other maintenance.

1.2. test description

I tested the old 0.7 vs. the current 0.95 with permutations of values (existing / non-existing & absolute / relative paths) for source and as-path (for cli-options and config-entries, including the absence of each or both).
While testing I made sure to exclude the repository by "globbing it out".

2. comment from 25.04.24

2.1. cli-issue (powershell)

The powershell-issue mentioned previously, looks like to be fixed by newer pwsh versions (which wasn't installed with windows 10).
So this should behave like bash when updated.

2.2. ERROR: "[...]a value is required for '--as-path[...]"

Under linux (with bash and v0.7 & v0.95): Specifying --as-path '' produces the ("correct") error-message mentioned under 2.2..
This error is probably related to clap::value_parser!(PathBuf) and it's policy to not allow 'empty' (='') paths.
Empty strings are totally valid for clap otherwise.
So collecting the string by clap first and "parsing" the path afterwards could do the trick, if consistency of cli-option --as-path and the config-option is desired.

3. WARNING: "[...]ignoring error - reading parent snapshot: TreeStackEmptyError"

For v0.95 the usage of '.' or './' as argument of source (either cli-option or config-entry) - and without(!) specifying as-path (both cli-option and config-entry) - the warning mentioned under 3. is shown.
The old v0.7 executes without warning.


not related to this issue but noticed while testing:

4. clarity/readability of warnings/errors

Some messages might be hard to understand or could probably be shorter for (especially non-dev) users.

4.1. examples:

• see heading 3.
• warning shown, when source is non-existent relative path:

"[...] ignoring error: Error: Failed to get next entry from walk iterator. [...] IO error [...] No such file or directory (os error 2)"

• error shown, when source (cli-argument) is non-existing absolute path:

"[...]canonicalizing path failed [...], message: "No such file or directory" [...]""

• error shown, when source (config-value) is non-existing absolute path:

"[...]error sanitizing sources=[...] in config file"

Thank you for your awesome work and have a great day!