From c75421238ad18b98c3ff16f43035ce264dc33e9d Mon Sep 17 00:00:00 2001 From: DeathAxe Date: Sun, 18 Dec 2016 21:53:26 +0100 Subject: [PATCH] Fix: Run all git commands from within working tree (Issue #239, #248, #290) Issue #239: This commit does not yet fix case sensitive issue with `git show -- REF:file` but helps git to respect config at all. Issue #248: A linked work-tree's root directory contains a file called '.git' which contains the path to the native repository. This seems not to work reliable on some platforms paired with `--git-dir` or `--work-tree` arguments. Issue #290: Same as with #248. GitGutter currently uses `--git-dir` and `--work-tree` arguments to pass the required information about the repository and its work-tree to git, but it seems git doesn't handle those arguments reliably. Commands running smoothly executed from shell in context of the working directory, fail when called by GitGutter. Either git output is empty or at least settings or `.gitattributes` are ignored. The behavior changes from call to call. One example is `git status`, which fails the first time when called with `--git-dir` and `--work-tree` and current directory being something outside the working tree. (Tested with Windows x64 / git 2.11.0) To avoid any trouble with git's commands/features the work-tree path is passed to `POpen` as `cwd` to perform a chdir to it before spawning git process. This makes `--git-dir` and `--work-tree` arguments obsolete as git finds everything it needs to be happy on its own. This is the way other plugins handle git execution, too. The `git_handler.work_tree()` method replaces `on_disk()` and checks, if the current view's file is part of a valid git repository. One of the parent directories must therefore contain a folder or file called `.git`, but the file path itself must not contain `.git` as this indicates a file within the database or something like COMMIT_EDITMSG file which does not belong to the work-tree and therefore must not be handled by GitGutter. The `git_dir` attribute is no longer needed and therefore removed as `_git_tree` is used as primary key for all required settings. In the end those changes made `git_helper` module obsolete. GitGutter detects renamed files now. 1. If the file is renamed within a work-tree its state changes to untracked. 2. If it is no longer part of a valid work-tree the state is completely cleared. 3. By moving it back into its original position the state is restored. Same works by removing and restoring the `.git` directory. Means, if you call `git init` in the open file's directory, GitGutter will update correctly. --- git_gutter.py | 18 +++-- git_gutter_handler.py | 146 +++++++++++++++++++++++----------------- git_gutter_settings.py | 14 ++-- git_gutter_show_diff.py | 7 +- git_helper.py | 47 ------------- 5 files changed, 111 insertions(+), 121 deletions(-) delete mode 100644 git_helper.py diff --git a/git_gutter.py b/git_gutter.py index d43bbabe..2e3797d1 100644 --- a/git_gutter.py +++ b/git_gutter.py @@ -27,6 +27,8 @@ def __init__(self, *args, **kwargs): TextCommand.__init__(self, *args, **kwargs) self.git_handler = GitGutterHandler(self.view) self.show_diff_handler = GitGutterShowDiff(self.view, self.git_handler) + # Last enabled state for change detection + self._enabled = False def is_enabled(self, **kwargs): """Determine if `git_gutter` command is _enabled to execute.""" @@ -50,14 +52,18 @@ def is_enabled(self, **kwargs): # Don't handle binary files elif view.encoding() in ('Hexadecimal'): valid = False - # Don't handle views without valid file - elif not self.git_handler.on_disk(): - valid = False # Don't handle files outside a repository - elif not self.git_handler.git_dir: + elif not self.git_handler.work_tree(validate=True): valid = False - # Save state for use in other modules - view.settings().set('git_gutter_enabled', valid) + # Handle changed state + if valid != self._enabled: + # File moved out of work-tree or repository gone + if not valid: + self.show_diff_handler.clear() + # Save state for use in other modules + view.settings().set('git_gutter_enabled', valid) + # Save state for internal use + self._enabled = valid return valid def run(self, edit, **kwargs): diff --git a/git_gutter_handler.py b/git_gutter_handler.py index 1ae6c53a..975eb238 100644 --- a/git_gutter_handler.py +++ b/git_gutter_handler.py @@ -9,11 +9,9 @@ import sublime try: - from . import git_helper from .git_gutter_settings import settings from .promise import Promise except (ImportError, ValueError): - import git_helper from git_gutter_settings import settings from promise import Promise @@ -37,9 +35,12 @@ def __init__(self, view): self.git_temp_file = None self.buf_temp_file = None - self.git_tree = None - self.git_dir = None - self.git_path = None + # cached view file name to detect renames + self._view_file_name = None + # real path to current work tree + self._git_tree = None + # relative file path in work tree + self._git_path = None self.git_tracked = False self._last_refresh_time_git_file = 0 @@ -61,6 +62,67 @@ def tmp_file(): os.close(fd) return filepath + @property + def repository_name(self): + """Return the folder name of the working tree as repository name.""" + return os.path.basename( + self._git_tree) if self._git_tree else '(None)' + + def work_tree(self, validate=False): + """Return the real path of a valid work-tree or None. + + Arguments: + validate (bool): If True check whether the file is part of a valid + git repository or return the cached working tree + path only on False. + """ + def is_work_tree(path): + """Return True if `path` contains a `.git` directory or file.""" + return path and os.path.exists(os.path.join(path, '.git')) + + def split_work_tree(file_path): + """Split the `file_path` into working tree and relative path. + + The file_path is converted to a absolute real path and split into + the working tree part and relative path part. + + Note: + This is a local alternitive to calling the git command: + + git rev-parse --show-toplevel + + Arguments: + file_path (string): full path to a file. + + Returns: + A tuble of two the elements (working tree, file path). + """ + if file_path: + path, name = os.path.split(os.path.realpath(file_path)) + # files within '.git' path are not part of a work tree + while path and name and name != '.git': + if is_work_tree(path): + return (path, os.path.relpath( + file_path, path).replace('\\', '/')) + path, name = os.path.split(path) + return (None, None) + + if validate: + # Check if file exists + file_name = self.view.file_name() + if not file_name or not os.path.isfile(file_name): + self._view_file_name = None + self._git_tree = None + self._git_path = None + return None + # Check if file was renamed + is_renamed = file_name != self._view_file_name + if is_renamed or not is_work_tree(self._git_tree): + self._view_file_name = file_name + self._git_tree, self._git_path = split_work_tree(file_name) + self.clear_git_time() + return self._git_tree + def git_time_cleared(self): return self._last_refresh_time_git_file == 0 @@ -75,7 +137,7 @@ def git_time(self): def get_compare_against(self): """Return the branch/commit/tag string the view is compared to.""" - return settings.get_compare_against(self.git_dir, self.view) + return settings.get_compare_against(self._git_tree, self.view) def set_compare_against(self, commit, refresh=False): """Apply a new branch/commit/tag string the view is compared to. @@ -91,7 +153,7 @@ def set_compare_against(self, commit, refresh=False): git show-ref refresh - always call git_gutter command """ - settings.set_compare_against(self.git_dir, commit) + settings.set_compare_against(self._git_tree, commit) self.clear_git_time() if refresh or not any(settings.get(key, True) for key in ( 'focus_change_mode', 'live_mode')): @@ -136,17 +198,6 @@ def in_repo(self): """ return self.git_tracked - def on_disk(self): - """Determine, if the view is saved to disk.""" - file_name = self.view.file_name() - on_disk = file_name is not None and os.path.isfile(file_name) - if on_disk: - self.git_tree = self.git_tree or git_helper.git_tree(self.view) - self.git_dir = self.git_dir or git_helper.git_dir(self.git_tree) - self.git_path = self.git_path or git_helper.git_file_path( - self.view, self.git_tree) - return on_disk - def update_buf_file(self): """Write view's content to temporary file as source for git diff.""" # Read from view buffer @@ -183,20 +234,12 @@ def write_file(contents): args = [ settings.git_binary_path, - '--git-dir=' + self.git_dir, - '--work-tree=' + self.git_tree, 'show', '%s:%s' % ( self.get_compare_against(), - self.git_path), + self._git_path), ] - - try: - self.update_git_time() - return GitGutterHandler.run_command( - args=args, decode=False).then(write_file) - except Exception: - pass + return self.run_command(args=args, decode=False).then(write_file) return Promise.resolve() def process_diff(self, diff_str): @@ -261,13 +304,8 @@ def run_diff(_unused): self.buf_temp_file, ] args = list(filter(None, args)) # Remove empty args - return GitGutterHandler.run_command( - args=args, decode=False).then(decode_diff) - - if self.on_disk() and self.git_path: - return self.update_git_file().then(run_diff) - else: - return Promise.resolve("") + return self.run_command(args=args, decode=False).then(decode_diff) + return self.update_git_file().then(run_diff) def process_diff_line_change(self, line_nr, diff_str): hunk_re = '^@@ \-(\d+),?(\d*) \+(\d+),?(\d*) @@' @@ -368,7 +406,7 @@ def ignored(self): def handle_files(self, additional_args): """Run git ls-files to check for untracked or ignored file.""" - if self.on_disk() and self.git_path: + if self._git_tree: def is_nonempty(results): """Determine if view's file is in git's index. @@ -379,14 +417,12 @@ def is_nonempty(results): args = [ settings.git_binary_path, - '--git-dir=' + self.git_dir, - '--work-tree=' + self.git_tree, 'ls-files', '--other', '--exclude-standard', ] + additional_args + [ - os.path.join(self.git_tree, self.git_path), + os.path.join(self._git_tree, self._git_path), ] args = list(filter(None, args)) # Remove empty args - return GitGutterHandler.run_command(args).then(is_nonempty) + return self.run_command(args).then(is_nonempty) return Promise.resolve(False) def git_commits(self): @@ -399,13 +435,11 @@ def git_commits(self): """ args = [ settings.git_binary_path, - '--git-dir=' + self.git_dir, - '--work-tree=' + self.git_tree, 'log', '--all', '--pretty=%h %s\a%an <%aE>\a%ad (%ar)', '--date=local', '--max-count=9000' ] - return GitGutterHandler.run_command(args) + return self.run_command(args) def git_file_commits(self): r"""Query all commits with changes to the attached file. @@ -418,51 +452,42 @@ def git_file_commits(self): """ args = [ settings.git_binary_path, - '--git-dir=' + self.git_dir, - '--work-tree=' + self.git_tree, 'log', '--pretty=%at\a%h %s\a%an <%aE>\a%ad (%ar)', '--date=local', '--max-count=9000', - '--', self.git_path + '--', self._git_path ] - return GitGutterHandler.run_command(args) + return self.run_command(args) def git_branches(self): args = [ settings.git_binary_path, - '--git-dir=' + self.git_dir, - '--work-tree=' + self.git_tree, 'for-each-ref', '--sort=-committerdate', '--format=%(subject)\a%(refname)\a%(objectname)', 'refs/heads/' ] - return GitGutterHandler.run_command(args) + return self.run_command(args) def git_tags(self): args = [ settings.git_binary_path, - '--git-dir=' + self.git_dir, - '--work-tree=' + self.git_tree, 'show-ref', '--tags', '--abbrev=7' ] - return GitGutterHandler.run_command(args) + return self.run_command(args) def git_current_branch(self): args = [ settings.git_binary_path, - '--git-dir=' + self.git_dir, - '--work-tree=' + self.git_tree, 'rev-parse', '--abbrev-ref', 'HEAD' ] - return GitGutterHandler.run_command(args) + return self.run_command(args) - @staticmethod - def run_command(args, decode=True): + def run_command(self, args, decode=True): """Run a git command asynchronously and return a Promise. Arguments: @@ -479,8 +504,9 @@ def read_output(resolve): else: startupinfo = None proc = subprocess.Popen( - args=args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, startupinfo=startupinfo) + args=args, cwd=self._git_tree, startupinfo=startupinfo, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + stdin=subprocess.PIPE) if _HAVE_TIMEOUT: stdout, stderr = proc.communicate(timeout=30) else: diff --git a/git_gutter_settings.py b/git_gutter_settings.py index 4825c5f2..fc97b26f 100644 --- a/git_gutter_settings.py +++ b/git_gutter_settings.py @@ -106,7 +106,7 @@ def load_settings(self): self._user_settings.get('show_in_minimap') or self._settings.get('show_in_minimap')) - def get_compare_against(self, git_dir, view): + def get_compare_against(self, git_tree, view): """Return the compare target for a view. If interactivly specified a compare target for the view's repository, @@ -115,26 +115,26 @@ def get_compare_against(self, git_dir, view): fall back to HEAD if everything goes wrong to avoid exceptions. Arguments: - git_dir - path of the `.git` directory holding the index + git_tree - real root path of the current work-tree view - the view whose settings to query first """ # Interactively specified compare target overrides settings. - if git_dir in self._compare_against_mapping: - return self._compare_against_mapping[git_dir] + if git_tree in self._compare_against_mapping: + return self._compare_against_mapping[git_tree] # Project settings and Preferences override plugin settings if set. compare = view.settings().get('git_gutter_compare_against') if not compare: compare = self.get('compare_against', 'HEAD') return compare - def set_compare_against(self, git_dir, new_compare_against): + def set_compare_against(self, git_tree, new_compare_against): """Assign a new compare target for current repository. Arguments: - git_dir - path of the .git directory holding the index + git_tree - real root path of the current work-tree new_compare_against - new branch/tag/commit to cmpare against """ - self._compare_against_mapping[git_dir] = new_compare_against + self._compare_against_mapping[git_tree] = new_compare_against @property def default_theme_path(self): diff --git a/git_gutter_show_diff.py b/git_gutter_show_diff.py index 615465b7..757eec4a 100644 --- a/git_gutter_show_diff.py +++ b/git_gutter_show_diff.py @@ -29,6 +29,11 @@ def __init__(self, view, git_handler): self.diff_results = None self.show_untracked = False + def clear(self): + """Remove all gutter icons and status messages.""" + self.view.erase_status('00_git_gutter') + self._clear_all() + def run(self): """Run diff and update gutter icons and status message.""" @@ -108,7 +113,7 @@ def set_status(branch_name): if template: # render the template using jinja2 library text = jinja2.environment.Template(''.join(template)).render( - repo=os.path.basename(self.git_handler.git_tree), + repo=self.git_handler.repository_name, compare=self.git_handler.format_compare_against(), branch=branch_name, state=file_state, deleted=len(deleted), inserted=len(inserted), modified=len(modified)) diff --git a/git_helper.py b/git_helper.py deleted file mode 100644 index 2cbd50d5..00000000 --- a/git_helper.py +++ /dev/null @@ -1,47 +0,0 @@ -import os - - -def git_file_path(view, git_path): - if not git_path: - return False - full_file_path = os.path.realpath(view.file_name()) - git_path_to_file = full_file_path.replace(git_path, '').replace('\\', '/') - if git_path_to_file[0] == '/': - git_path_to_file = git_path_to_file[1:] - return git_path_to_file - - -def git_root(directory): - if os.path.exists(os.path.join(directory, '.git')): - return directory - else: - parent = os.path.realpath(os.path.join(directory, os.path.pardir)) - if parent == directory: - # we have reached root dir - return False - else: - return git_root(parent) - - -def git_tree(view): - full_file_path = view.file_name() - file_parent_dir = os.path.realpath(os.path.dirname(full_file_path)) - return git_root(file_parent_dir) - - -def git_dir(directory): - if not directory: - return False - pre_git_dir = os.path.join(directory, '.git') - if os.path.isfile(pre_git_dir): - submodule_path = '' - with open(pre_git_dir) as submodule_git_file: - submodule_path = submodule_git_file.read() - submodule_path = os.path.join('..', submodule_path.split()[1]) - - submodule_git_dir = os.path.abspath( - os.path.join(pre_git_dir, submodule_path)) - - return submodule_git_dir - else: - return pre_git_dir