Skip to content

Enable resolution of variables in config paths - #316

Open
oWretch wants to merge 2 commits into
alichtman:mainfrom
oWretch:master
Open

Enable resolution of variables in config paths#316
oWretch wants to merge 2 commits into
alichtman:mainfrom
oWretch:master

Conversation

@oWretch

@oWretch oWretch commented Jan 2, 2023

Copy link
Copy Markdown

I have a desire to sync my configuration between machines where my username is different on each. Currently, the config backup just uses the raw path provided in the configuration file and doesn't attempt to resolve any variables from within the path.

This PR updates the config backup process to resolve variables in the config path string, such as ~ and $HOME and thereby allow one config to be used across different machines.

The upshot of this change is it would be possible for someone to perform advanced backup and restore by setting environment variables to move files around between machines.

@alichtman

alichtman commented Jan 24, 2023

Copy link
Copy Markdown
Owner

Hey! Thanks for the PR :) This fix makes sense to me.

I think it makes sense to break os.path.expandvars(os.path.expanduser(config_path)) out into a utility function. There are likely more places in the codebase we'd like to use that function. Once that change is made, this seems good to go.

EDIT

Turns out this code is already a util function:

def expand_to_abs_path(path):
"""
Expands relative and user's home paths to the respective absolute path. Environment
variables found on the input path will also be expanded.
:param path: Path to be expanded.
:return: (str) The absolute path.
"""
expanded_path = os.path.expanduser(path)
expanded_path = os.path.expandvars(expanded_path)
return os.path.abspath(expanded_path)

Let's use expand_to_abs_path() here.

@alichtman
alichtman self-requested a review January 24, 2023 10:37
Comment thread shallow_backup/backup.py Outdated
Comment thread shallow_backup/backup.py
@@ -114,12 +114,13 @@ def backup_configs(backup_path, dry_run: bool = False, skip=False):
continue

quoted_dest = quote(dest)

@alichtman alichtman Jan 24, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is also a problem for dest / quoted_dest.

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.

2 participants