diff --git a/README.md b/README.md index d64702aa..3aa1397c 100644 --- a/README.md +++ b/README.md @@ -281,6 +281,11 @@ msync update --noop msync update -m "Commit message" ``` +Use `--rebase` to update the local working branch with the latest remote default +branch before ModuleSync changes any files. If the rebase conflicts, ModuleSync +aborts the rebase and reports an error for that module. A branch changed by the +rebase is pushed even when the template sync itself produces no changes. + Available parameters for modulesync.yml * `git_base` : The default URL to git clone from (Default: 'git@github.com:') diff --git a/lib/modulesync.rb b/lib/modulesync.rb index 4e3c6a4d..76e7b504 100644 --- a/lib/modulesync.rb +++ b/lib/modulesync.rb @@ -123,7 +123,8 @@ def self.manage_module(puppet_module, module_files, defaults) # but we totally skip the workspace preparation to keep the current behavior unless options[:offline] puppet_module.repository.prepare_workspace(branch: options[:branch], - operate_offline: false) + operate_offline: false, + rebase: options[:rebase]) end module_configs = Util.parse_config puppet_module.path(MODULE_CONF_FILE) diff --git a/lib/modulesync/cli.rb b/lib/modulesync/cli.rb index f3723336..6e7c0428 100644 --- a/lib/modulesync/cli.rb +++ b/lib/modulesync/cli.rb @@ -114,6 +114,10 @@ class Base < Thor type: :boolean, desc: 'Do not run any Git commands. Allows the user to manage Git outside of ModuleSync.', default: false + option :rebase, + type: :boolean, + desc: 'Rebase the local branch onto the latest remote default branch before syncing.', + default: false option :bump, type: :boolean, desc: 'Bump module version to the next minor', diff --git a/lib/modulesync/repository.rb b/lib/modulesync/repository.rb index d3dfa0d0..92859572 100644 --- a/lib/modulesync/repository.rb +++ b/lib/modulesync/repository.rb @@ -42,6 +42,12 @@ def remote_branch_ahead?(source_branch, target_branch) commits.any? end + def branch_behind?(branch, target_branch) + log = repo.log(1).between(branch, "origin/#{target_branch}") + commits = log.respond_to?(:execute) ? log.execute : log + commits.any? + end + def default_branch # `Git.default_branch` requires ruby-git >= 1.17.0 return Git.default_branch(repo.dir) if Git.respond_to? :default_branch @@ -52,6 +58,12 @@ def default_branch %r{remotes/origin/HEAD\s+->\s+origin/(?.+?)$}.match(symbolic_ref.full)[:branch] end + def remote_default_branch + return Git.default_branch(@remote) if Git.respond_to? :default_branch + + default_branch + end + def switch(branch:) unless branch branch = default_branch @@ -87,13 +99,16 @@ def clone @git = Git.clone(@remote, @directory) end - def prepare_workspace(branch:, operate_offline:) + def prepare_workspace(branch:, operate_offline:, rebase: false) + @rebased = false if cloned? puts "Overriding any local changes to repository in '#{@directory}'" git.fetch 'origin', prune: true unless operate_offline git.reset_hard + rebase_target = remote_default_branch if rebase && !operate_offline switch(branch: branch) git.pull('origin', branch) if !operate_offline && remote_branch_exists?(branch) + rebase_onto(rebase_target) if rebase_target else raise ModuleSync::Error, 'Unable to clone in offline mode.' if operate_offline @@ -102,6 +117,31 @@ def prepare_workspace(branch:, operate_offline:) end end + def rebase_onto(branch) + return false if repo.current_branch == branch || !branch_behind?(repo.current_branch, branch) + + puts "Rebasing #{repo.current_branch} onto origin/#{branch}" + repo.lib.send(:command, 'rebase', "origin/#{branch}") + @rebased = true + rescue Git::Error => e + begin + repo.lib.send(:command, 'rebase', '--abort') + rescue Git::Error + # Preserve the original rebase error if Git had no rebase to abort. + end + raise ModuleSync::Error, "Rebase onto origin/#{branch} failed and was aborted: #{e.message}" + end + + def push_changes(branch, remote_branch, options) + refspec = remote_branch ? "#{branch}:#{remote_branch}" : branch + if @rebased && !options[:force] + repo.lib.send(:command, 'push', '--force-with-lease', 'origin', refspec) + else + opts_push = options[:force] ? { force: true } : {} + repo.push('origin', refspec, opts_push) + end + end + def default_reset_branch(branch) remote_branch_exists?(branch) ? branch : default_branch end @@ -151,26 +191,24 @@ def submit_changes(files, options) end begin opts_commit = {} - opts_push = {} opts_commit = { amend: true } if options[:amend] - opts_push = { force: true } if options[:force] if options[:pre_commit_script] script = "#{File.dirname(__FILE__, 3)}/contrib/#{options[:pre_commit_script]}" system(script, @directory) end if repo.status.changed.empty? && repo.status.added.empty? && repo.status.deleted.empty? puts "There were no changes in '#{@directory}'. Not committing." - return false + return false unless @rebased else repo.commit(message, opts_commit) end if options[:remote_branch] - if remote_branch_differ?(branch, options[:remote_branch]) - repo.push('origin', "#{branch}:#{options[:remote_branch]}", opts_push) + if @rebased || remote_branch_differ?(branch, options[:remote_branch]) + push_changes(branch, options[:remote_branch], options) puts "Changes have been pushed to: '#{branch}:#{options[:remote_branch]}'" end else - repo.push('origin', branch, opts_push) + push_changes(branch, nil, options) puts "Changes have been pushed to: '#{branch}'" end rescue Git::Error => e diff --git a/spec/unit/module_sync/repository_spec.rb b/spec/unit/module_sync/repository_spec.rb index c3a7c7d1..774cd8fe 100644 --- a/spec/unit/module_sync/repository_spec.rb +++ b/spec/unit/module_sync/repository_spec.rb @@ -15,10 +15,12 @@ end let(:branches) { instance_double(Git::Branches, remote: remote_branches) } let(:log) { instance_double(Git::Log) } - let(:git) { instance_double(Git::Base, branches: branches, log: log) } + let(:git_lib) { instance_double(Git::Lib) } + let(:git) { instance_double(Git::Base, branches: branches, lib: git_lib, log: log) } before do repository.instance_variable_set(:@git, git) + ModuleSync.instance_variable_set(:@options, verbose: false) end it 'detects commits on a remote source branch that are missing from the target branch' do @@ -33,4 +35,85 @@ expect(repository.remote_branch_ahead?('modulesync', 'main')).to be false end + + describe '#rebase_onto' do + before do + allow(git).to receive(:current_branch).and_return('modulesync') + allow(log).to receive(:between).with('modulesync', 'origin/main').and_return(log) + allow(log).to receive(:execute).and_return([instance_double(Git::Object::Commit)]) + end + + it 'rebases the current branch onto the remote default branch' do + expect(git_lib).to receive(:send).with(:command, 'rebase', 'origin/main') + + repository.rebase_onto('main') + end + + it 'does not rebase the default branch onto itself' do + allow(git).to receive(:current_branch).and_return('main') + expect(git_lib).not_to receive(:send) + + repository.rebase_onto('main') + end + + it 'does not rebase a branch that already contains the remote default branch' do + allow(log).to receive(:execute).and_return([]) + expect(git_lib).not_to receive(:send) + + repository.rebase_onto('main') + end + + it 'aborts and reports a failed rebase' do + allow(git_lib).to receive(:send).with(:command, 'rebase', 'origin/main').and_raise(Git::Error, 'merge conflict') + expect(git_lib).to receive(:send).with(:command, 'rebase', '--abort') + + expect { repository.rebase_onto('main') } + .to raise_error(ModuleSync::Error, %r{Rebase onto origin/main failed and was aborted: merge conflict}) + end + end + + describe '#prepare_workspace' do + it 'fetches and rebases the selected branch onto the remote default branch when requested' do + allow(Dir).to receive(:exist?).with('/tmp/example/.git').and_return(true) + allow(Git).to receive(:default_branch).with('https://github.com/example/repository.git').and_return('main') + allow(git).to receive(:current_branch).and_return('modulesync') + allow(log).to receive(:between).with('modulesync', 'origin/main').and_return(log) + allow(log).to receive(:execute).and_return([instance_double(Git::Object::Commit)]) + expect(git).to receive(:fetch).with('origin', prune: true).ordered + expect(git).to receive(:reset_hard).ordered + expect(git).to receive(:pull).with('origin', 'modulesync').ordered + expect(git_lib).to receive(:send).with(:command, 'rebase', 'origin/main').ordered + + repository.prepare_workspace(branch: 'modulesync', operate_offline: false, rebase: true) + end + end + + describe '#submit_changes' do + it 'does not push an unchanged branch that was not rebased' do + status = instance_double(Git::Status, added: {}, changed: {}, deleted: {}) + branch = instance_double(Git::Branch, checkout: true) + allow(git).to receive(:branch).with('modulesync').and_return(branch) + allow(git).to receive(:status).and_return(status) + expect(git).not_to receive(:push) + expect(git_lib).not_to receive(:send) + + result = repository.submit_changes([], branch: 'modulesync', message: 'Update', force: false) + + expect(result).to be false + end + + it 'pushes a rebased branch even when ModuleSync made no file changes' do + status = instance_double(Git::Status, added: {}, changed: {}, deleted: {}) + branch = instance_double(Git::Branch, checkout: true) + repository.instance_variable_set(:@rebased, true) + allow(git).to receive(:branch).with('modulesync').and_return(branch) + allow(git).to receive(:status).and_return(status) + expect(git_lib).to receive(:send) + .with(:command, 'push', '--force-with-lease', 'origin', 'modulesync') + + result = repository.submit_changes([], branch: 'modulesync', message: 'Update', force: false) + + expect(result).to be true + end + end end diff --git a/spec/unit/module_sync_spec.rb b/spec/unit/module_sync_spec.rb index 36f6a498..6d6f88a3 100644 --- a/spec/unit/module_sync_spec.rb +++ b/spec/unit/module_sync_spec.rb @@ -63,5 +63,12 @@ described_class.manage_module(puppet_module, [], {}) end + + it 'requests a rebase before syncing when enabled' do + described_class.instance_variable_set(:@options, described_class.config_defaults.merge(rebase: true)) + expect(repository).to receive(:prepare_workspace).with(branch: nil, operate_offline: false, rebase: true) + + described_class.manage_module(puppet_module, [], {}) + end end end