Fix Git extension /home hardcoding for root user (#337)#354
Fix Git extension /home hardcoding for root user (#337)#354strangerwhoisharborofdoom wants to merge 1 commit intoosrf:mainfrom
Conversation
The Git extension was hardcoding /home/%(username)s/.gitconfig which breaks when using the root user (whose home is /root instead of /home/root). This fix uses os.path.expanduser() to dynamically determine the correct home directory for any user, fixing issue osrf#337. Changes: - Replace hardcoded '/home/%(username)s/.gitconfig' with dynamic path - Uses os.path.expanduser('~' + username) for proper home directory resolution - Works correctly for root user and all other users
There was a problem hiding this comment.
Pull request overview
This PR aims to fix the git extension’s assumption that a user’s home directory is always under /home, which breaks when running as root (home is /root).
Changes:
- Update
git_extension.pyto derive the container-side~/.gitconfigmount target dynamically (intended to address issue #337).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| username = getpass.getuser() | ||
| if 'user_override_name' in cli_args and cli_args['user_override_name']: | ||
| username = cli_args['user_override_name'] | ||
| user_gitconfig_target = '/home/%(username)s/.gitconfig' % locals() | ||
| user_gitconfig_target = os.path.join(os.path.expanduser('~' + username) if username else os.path.expanduser('~'), '.gitconfig') # fixes #337 |
There was a problem hiding this comment.
os.path.expanduser('~' + username) resolves the home directory using the host user database. For override names like testusername (used in the existing tests), this commonly won’t exist on the host, so expanduser will return the literal ~testusername and produce an invalid container target path. To stay consistent with the user extension behavior (extensions.py sets the container home to /home/<override> for overrides), derive user_gitconfig_target from that same rule (e.g., /home/<override>/.gitconfig, with a special-case for root -> /root/.gitconfig) rather than calling expanduser on an arbitrary username.
| user_gitconfig_target = '/home/%(username)s/.gitconfig' % locals() | ||
| user_gitconfig_target = os.path.join(os.path.expanduser('~' + username) if username else os.path.expanduser('~'), '.gitconfig') # fixes #337 | ||
|
|
||
| ) |
There was a problem hiding this comment.
There is a stray closing parenthesis here (line 42) with no matching opening paren, which makes this file a SyntaxError and will prevent the extension from loading. Remove the extraneous ')' (and re-check indentation/block structure) so get_docker_args() parses correctly.
| ) |
| if 'user' in cli_args and cli_args['user']: | ||
| username = getpass.getuser() | ||
| if 'user_override_name' in cli_args and cli_args['user_override_name']: | ||
| username = cli_args['user_override_name'] | ||
| user_gitconfig_target = '/home/%(username)s/.gitconfig' % locals() | ||
| user_gitconfig_target = os.path.join(os.path.expanduser('~' + username) if username else os.path.expanduser('~'), '.gitconfig') # fixes #337 | ||
|
|
There was a problem hiding this comment.
Inside the user_override_name branch, the code no longer assigns username = cli_args['user_override_name'], so username remains getpass.getuser() and the override has no effect. Also, user_gitconfig_target is now only set when user_override_name is truthy; when --user is enabled without an override it stays /root/.gitconfig, which breaks the existing behavior (and the unit test expectations in test/test_git_extension.py). Restore the prior control flow: compute username first (considering overrides) and set user_gitconfig_target for all --user cases.
The Git extension was hardcoding /home/%(username)s/.gitconfig which breaks when using the root user (whose home is /root instead of /home/root).
This fix uses os.path.expanduser() to dynamically determine the correct home directory for any user, fixing issue #337.
Changes: