Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix command line arguments for duplicity 2.1 #2301

Merged
merged 3 commits into from Sep 2, 2023

Conversation

dms00
Copy link

@dms00 dms00 commented Aug 29, 2023

Duplicity v2.1.0 backups are failing with the error:
"... --gpg-options expected one argument".

The issue is that duplicity v2.1.0 began using the argparse Python
library and the parse_known_args function. This function
interprets the argument being passed, "--cipher-algo=AES256",
as an argument name (because of the leading '-') and not as an
argument value. Because of that it exits with an error and
reports that the --gpg-options arg is missing its value.

Adding an extra set of quotes around this string causes
parse_known_args to interpret the string as an argument
value.

@dms00
Copy link
Author

dms00 commented Aug 29, 2023

This error started happening on my system with mailinabox v63, running duplicity version 2.1.0. That is where this fix has been tested. I do not know if the fix works on the older version of duplicity, namely 1.2.3.

@JoshData
Copy link
Member

Thanks.

interpreted by shell

Despite the function name, there is no shell involved in launching duplicity. The Mail-in-a-Box Python process invokes the duplicity process directly. So that can't be the explanation. But duplicity could be interpreting single quotes in a special/unusual way.

@dms00
Copy link
Author

dms00 commented Aug 30, 2023

Thanks for the information on the shell function. With that in mind, I took a closer look at duplicity. Turns out the way it processes args has changed completely between 1.2.3 and 2.1.0. In 1.2.3 they use optparse (which I've personally never used), and in 2.1.0 they converted to using argparse. Perhaps this is the explanation for why the arguments are being treated differently.

@dms00
Copy link
Author

dms00 commented Aug 30, 2023

@JoshData you were right about it handling quotes, as I just found here when digging a little deeper: https://gitlab.com/duplicity/duplicity/-/blob/rel.2.1.0/duplicity/cli_util.py?ref_type=tags#L108
What I still don't quite understand is why it does not work without the quotes.

@dms00
Copy link
Author

dms00 commented Aug 30, 2023

After quite a bit more research on this (and experimentation using duplicity 2.1.0), I've determined this is where the code is exiting with an error:
https://gitlab.com/duplicity/duplicity/-/blob/rel.2.1.0/duplicity/cli_main.py?ref_type=tags#L139

When I realized argparse's parse_known_args was failing, I did some experimentation in a python repl and it's pretty clear what's going on. I'm not sure if this is a the behavior the duplicity folks intended or not, so I'm not saying this is a bug, but here's a simple demonstration of why this won't work without the quotes:

First, without quotes:
>>> import argparse
>>> parser = argparse.ArgumentParser()
>>> parser.add_argument('--foo')
_StoreAction(option_strings=['--foo'], dest='foo', nargs=None, const=None, default=None, type=None, choices=None, required=False, help=None, metavar=None)
>>> parser.parse_known_args(['--foo', '--badger'])
usage: [-h] [--foo FOO]
: error: argument --foo: expected one argument

Then with quotes:

>>> import argparse
>>> parser = argparse.ArgumentParser()
>>> parser.add_argument('--foo')
_StoreAction(option_strings=['--foo'], dest='foo', nargs=None, const=None, default=None, type=None, choices=None, required=False, help=None, metavar=None)
>>> parser.parse_known_args(['--foo', '"--badger"'])
(Namespace(foo='"--badger"'), [])

@JoshData
Copy link
Member

Ahha now it makes perfect sense. Seems like with argparse, arguments that look like argument names because they start with a dash have to be quoted to be interpreted as an argument value. Totally sensible.

Now that I understand it, I'm happy to merge the PR. (Although if you wouldn't mind updating the commit description (using commit with --amend and push with --force) with the new understanding of the problem that'll save me a step.)

(Since Mail-in-a-Box always upgrades installed apt packages during installation, we can assume that the current/latest version of duplicity is installed with this code, so we don't have to worry about backwards compatibility.)

Duplicity v2.1.0 backups are failing with the error:
"... --gpg-options expected one argument".

The issue is that duplicity v2.1.0 began using the argparse Python
library and the parse_known_args function. This function
interprets the argument being passed, "--cipher-algo=AES256",
as an argument name (because of the leading '-') and not as an
argument value. Because of that it exits with an error and
reports that the --gpg-options arg is missing its value.

Adding an extra set of quotes around this string causes
parse_known_args to interpret the string as an argument
value.
@dms00
Copy link
Author

dms00 commented Aug 30, 2023

Okay, this is my first force push while working with a fork, so I hope I got it right. If it's not correct, please let me know and I'll try to work it out.

@jvolkenant
Copy link
Contributor

jvolkenant commented Aug 31, 2023

Looks like Duplicity 2.1.0 was updated recently. In addition to the quoting above, I had to do the following:

  • Move source and target positional arguments to the end
  • Add quote to the returned lists in 'get_duplicity_additional_args()'

See fdcce08 for those changes

I tested all arguments except for --restore, I don't have enough space to test that atm

When someone tests this MR, let me know if you also needed these changes

@dms00 dms00 changed the title Fix how the arg is being passed to gpg-options parameter Fix how the value is being passed for the gpg-options parameter Aug 31, 2023
@johnjaylward
Copy link

This appears to fix the nightly backup job, but I'm still having errors in the admin UI:

image

I'm not sure which logs to check to see what the actual error is...

@johnjaylward
Copy link

nevermind, found my issue. I restarted nginx, but I was supposed to restart the actual mailinabox service. once I restarted that, this fixed the issue in both the UI and the nightly

@JoshData
Copy link
Member

JoshData commented Sep 2, 2023

@jvolkenant Thanks! I've added your changes to this PR. Everything is working OK for me, so I will merge. Thanks all. :)

@JoshData JoshData changed the title Fix how the value is being passed for the gpg-options parameter Fix command line arguments for duplicity 2.1 Sep 2, 2023
@JoshData JoshData merged commit a966913 into mail-in-a-box:main Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants