diff --git a/nbgitpuller/pull.py b/nbgitpuller/pull.py index be379902..2b74b4b4 100644 --- a/nbgitpuller/pull.py +++ b/nbgitpuller/pull.py @@ -244,6 +244,62 @@ def rename_local_untracked(self): os.rename(f, new_file_name) yield 'Renamed {} to {} to avoid conflict with upstream'.format(f, new_file_name) + def merge(self): + """ + Merges branch from origin into current branch, resolving conflicts when possible. + + Resolves conflicts in two ways: + + - Passes `-Xours` to git, setting merge-strategy to preserve changes made by the + user whererver possible + - Detect (modify/delete) conflicts, where the user has locally modified something + that was deleted upstream. We just keep the local file. + """ + modify_delete_conflict = False + try: + for line in execute_cmd([ + 'git', + '-c', 'user.email=nbgitpuller@nbgitpuller.link', + '-c', 'user.name=nbgitpuller', + 'merge', + '-Xours', 'origin/{}'.format(self.branch_name) + ], + cwd=self.repo_dir): + yield line + # Detect conflict caused by one branch + if line.startswith("CONFLICT (modify/delete)"): + modify_delete_conflict = True + except subprocess.CalledProcessError: + if not modify_delete_conflict: + raise + + if modify_delete_conflict: + yield "Caught modify/delete conflict, trying to resolve" + # If a file was deleted on one branch, and modified on another, + # we just keep the modified file. This is done by `git add`ing it. + yield from self.commit_all() + + def commit_all(self): + """ + Creates a new commit with all current changes + """ + yield from execute_cmd([ + 'git', + # We explicitly set user info of the commits we are making, to keep that separate from + # whatever author info is set in system / repo config by the user. We pass '-c' to git + # itself (rather than to 'git commit') to temporarily set config variables. This is + # better than passing --author, since git treats author separately from committer. + '-c', 'user.email=nbgitpuller@nbgitpuller.link', + '-c', 'user.name=nbgitpuller', + 'commit', + '-am', 'Automatic commit by nbgitpuller', + # We allow empty commits. On NFS (at least), sometimes repo_is_dirty returns a false + # positive, returning True even when there are no local changes (git diff-files seems to return + # bogus output?). While ideally that would not happen, allowing empty commits keeps us + # resilient to that issue. + '--allow-empty' + ], cwd=self.repo_dir) + def update(self): """ Do the pulling if necessary @@ -266,34 +322,13 @@ def update(self): yield from execute_cmd(['git', 'reset', '--mixed'], cwd=self.repo_dir) # If there are local changes, make a commit so we can do merges when pulling - # We also allow empty commits. On NFS (at least), sometimes repo_is_dirty returns a false - # positive, returning True even when there are no local changes (git diff-files seems to return - # bogus output?). While ideally that would not happen, allowing empty commits keeps us - # resilient to that issue. - # We explicitly set user info of the commits we are making, to keep that separate from - # whatever author info is set in system / repo config by the user. We pass '-c' to git - # itself (rather than to 'git commit') to temporarily set config variables. This is - # better than passing --author, since git treats author separately from committer. if self.repo_is_dirty(): yield from self.ensure_lock() - yield from execute_cmd([ - 'git', - '-c', 'user.email=nbgitpuller@nbgitpuller.link', - '-c', 'user.name=nbgitpuller', - 'commit', - '-am', 'Automatic commit by nbgitpuller', - '--allow-empty' - ], cwd=self.repo_dir) + yield from self.commit_all() # Merge master into local! yield from self.ensure_lock() - yield from execute_cmd([ - 'git', - '-c', 'user.email=nbgitpuller@nbgitpuller.link', - '-c', 'user.name=nbgitpuller', - 'merge', - '-Xours', 'origin/{}'.format(self.branch_name) - ], cwd=self.repo_dir) + yield from self.merge() def main(): diff --git a/tests/test_gitpuller.py b/tests/test_gitpuller.py index a7c222e4..b4d742aa 100644 --- a/tests/test_gitpuller.py +++ b/tests/test_gitpuller.py @@ -355,6 +355,85 @@ def test_delete_remotely_modify_locally(): assert puller.read_file('README.md') == 'HELLO' +def test_diverged(): + """ + Test deleting a file upstream, and editing it locally. This time we + commit to create diverged brances. + """ + with Remote() as remote, Pusher(remote) as pusher: + pusher.push_file('README.md', 'new') + + with Puller(remote) as puller: + # Delete the file remotely + pusher.git('rm', 'README.md') + pusher.git('commit', '-m', 'Deleted file') + pusher.git('push', '-u', 'origin', 'master') + + # Edit locally + puller.write_file('README.md', 'conflict') + puller.git('add', 'README.md') + puller.git('commit', '-m', 'Make conflicting change') + + # The local change should be kept + puller.pull_all() + assert puller.read_file('README.md') == 'conflict' + + +def test_diverged_reverse(): + """ + Test deleting a file locally, and editing it upstream. We commit the changes + to create diverged branches. Like `test_diverged`, but flipped. + """ + with Remote() as remote, Pusher(remote) as pusher: + pusher.push_file('README.md', 'new') + + with Puller(remote) as puller: + # Delete the file locally + puller.git('rm', 'README.md') + puller.git('commit', '-m', 'Deleted file') + + # Edit the file remotely + pusher.push_file('README.md', 'conflicting change') + + # Pulling should get the latest version of the file + puller.pull_all() + assert(puller.read_file('README.md') == 'conflicting change') + + +def test_diverged_multiple(): + """ + Test deleting a file upstream, and editing it locally. We commit the changes + to create diverged branches. + + Use two files, so git merge doesn't mention the conflict in the first line. + + puller: Auto-merging AFILE.txt + puller: CONFLICT (modify/delete): BFILE.txt deleted in origin/master and modified in HEAD. Version HEAD of BFILE.txt left in tree. + puller: Automatic merge failed; fix conflicts and then commit the result. + + """ + with Remote() as remote, Pusher(remote) as pusher: + pusher.push_file('AFILE.txt', 'new') + pusher.push_file('BFILE.txt', 'new') + + with Puller(remote) as puller: + # Remote changes - BFILE.txt deleted + pusher.write_file('AFILE.txt', 'changed remotely') + pusher.git('add', 'AFILE.txt') + pusher.git('rm', 'BFILE.txt') + pusher.git('commit', '-m', 'Remote changes') + pusher.git('push', '-u', 'origin', 'master') + + # Local changes - BFILE.txt edited + puller.write_file('AFILE.txt', 'edited') + puller.write_file('BFILE.txt', 'edited') + puller.git('commit', '-am', 'Make conflicting change') + + puller.pull_all() + assert puller.read_file('AFILE.txt') == 'edited' + assert puller.read_file('BFILE.txt') == 'edited' + + def test_delete_locally_and_remotely(): """ Test that sync works after deleting a file locally and remotely