-
Notifications
You must be signed in to change notification settings - Fork 142
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
logs: put borg arguments within quotation marks when they contain special strings #2190
base: master
Are you sure you want to change the base?
Conversation
@m3nu This is ready for review. |
Thanks for the PR, as always. So this aims to log a command for copy & pasting and reversing some of the escaping done when logging this? I wonder if there is a simpler way to achieve something similar? E.g. here some different ways to log stuff.
The issue could also come from our For this PR I worry that:
|
@m3nu: Thanks for the feedback. Printing the list of borg command arguments is a simple one-liner: So a simple (and correct) solution would be to just put this single line instead of (re-)constructing a valid shell command. The list of borg command arguments does not contain any escape characters or quotation marks. These are not required if no shell is involved.
Computational overhead: I wondered about this, too. :-) To get a rough estimate, I run the code to format the pretty long and complicated list of borg arguments from my commit message, on my mid-range laptop inside a Jupyter notebook. Complexity I agree that the code in the pull request is not self-explanatory, since I tried to put the first quotation mark after the first "=" if there are any.
The only difference for the shell command approximation would be that with the simplified code I would get Currently, in 0.10.3, we write a borg command (approximation) to the logs that escape whitespace using "\" and without any quotation marks (not even for regular expressions "re:") - I would like to fix this. I believe there are several valid options, here:
Any preferences? |
Thinking about this again, I believe option 2 would be a good one:
|
@m3nu I simplified the code and updated my pull request: Changes:
This solution is more robust than what is in 0.10.3:
Example log output with the modified pull request - for a complicated case: This command can be copied pasted from the logs to the shell - and runs just fine (only archive name has to be updated, obviously) As before: The execution path is not concerned here - this is purely about a pretty and useful (for debugging and copy) borg command in the logs. |
The new version is much clearer! No objections to add this. |
Description
This pull request has two purposes:
The pull request only concerns what is printed to the logs.
Related Issue
#2189
Current situation in Vorta (prior to 0.10.3):
When executing borg commands, an "approximation" of the borg (shell) command is printed to the logs.
This command is an approximation in the sense that it is artificially constructed from a list of command line arguments
cmd
, that is used internally by Vorta. In the past this lead to issues #2164, since the approximation of the borg command in the logs was correct, but internally the list of argumentscmd
was broken. :-/Background: The list of command line arguments
cmd
does neither require nor contain quotation marks - or escape characters (e.g. for special characters), and Vorta passescmd
"as is" to Popen() for execution.For shell usage, borg recommends the use of quotation marks in the following cases:
This pull request does two things:
cmd
to the logsMotivation and Context
cmd
, this makes debugging cumbersome. See Extra borg arguments with white space raise an error #2164How Has This Been Tested?
The following borg arguments have been added to the "extra borg arguments" via the Vorta UI - to check if the resulting shell command approximation provides the intended format / quotation marks:
These are the resulting entries in the logs:
The list of borg command line arguments
cmd
. Note that quotation marks have been stripped away:2025-01-13 12:35:57,302 - vorta.borg.borg_job - INFO - borg command arguments: ['/usr/bin/borg', 'create', '--remote-path=/usr/local/bin/borg', '--list', '--progress', '--info', '--log-json', '--json', '--filter=AM', '-C', 'lz4', '--pattern=+home/user/test folder/testfile', '--pattern=-home/user/test folder', '--pattern=-home/goebbe/test folder', '--pattern=-re:home/user/test\\sfolder2', '--pattern=-re:crazy?=cra', '-e', 're:crazy?=cra', '-e', '/home/user/*.tmp', '-e', '/home/user/test/cache', '--patterns-from', '/home/user/patterns.lst', 'ssh://user@192.168.0.5:22/~/backup/vorta-repo::user-2025-01-13-123557', '/home/user']
The shell command approximation:
2025-01-13 12:35:57,302 - vorta.borg.borg_job - INFO - shell command approximation: /usr/bin/borg create --remote-path=/usr/local/bin/borg --list --progress --info --log-json --json --filter=AM -C lz4 --pattern='+home/user/test folder/testfile' --pattern='-home/user/test folder' --pattern='-home/user/test folder' --pattern='-re:home/user/test\sfolder2' --pattern='-re:crazy?=cra' -e 're:crazy?=cra' -e '/home/user/*.tmp' -e /home/user/test/cache --patterns-from /home/user/patterns.lst ssh://user@192.168.0.5:22/~/backup/vorta-repo::user-2025-01-13-123557 /home/user
The shell command approximation can be copied to a shell and executed "as is".
Discussion:
Printing both, the list of command line arguments AND the shell command approximation, leads to obvious redundancies, but each of the options has its specific advantages:
List of borg command line arguments:
shell command approximation:
Personally, I would put both to the logs, since both are helpful in some cases.
Types of changes
Checklist:
I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.