From 102657a39b5cd89c7602d5df294e5b5e54e913c5 Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Sun, 7 Jan 2018 09:35:30 -0500 Subject: [PATCH 1/6] Honor a repository's default branch, which is automatically checked out when cloned. --- lib/modulesync/cli.rb | 4 ++-- lib/modulesync/git.rb | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/modulesync/cli.rb b/lib/modulesync/cli.rb index 451d884e..60c79623 100644 --- a/lib/modulesync/cli.rb +++ b/lib/modulesync/cli.rb @@ -53,8 +53,8 @@ class Base < Thor :desc => 'A regular expression to skip repositories.' class_option :branch, :aliases => '-b', - :desc => 'Branch name to make the changes in. Defaults to master.', - :default => CLI.defaults[:branch] || 'master' + :desc => 'Branch name to make the changes in. Defaults to the default branch of the upstream repository.', + :default => CLI.defaults[:branch] desc 'update', 'Update the modules in managed_modules.yml' option :message, diff --git a/lib/modulesync/git.rb b/lib/modulesync/git.rb index 843c6460..367dd07f 100644 --- a/lib/modulesync/git.rb +++ b/lib/modulesync/git.rb @@ -19,6 +19,10 @@ def self.remote_branch_differ?(repo, local_branch, remote_branch) end def self.switch_branch(repo, branch) + unless branch + puts "Using repository's default branch" + return + end return if repo.current_branch == branch if local_branch_exists?(repo, branch) @@ -100,6 +104,7 @@ def self.update(name, files, options) module_root = "#{options[:project_root]}/#{name}" message = options[:message] repo = ::Git.open(module_root) + options[:branch] ||= (repo.current_branch || 'master') repo.branch(options[:branch]).checkout files.each do |file| if repo.status.deleted.include?(file) @@ -154,6 +159,7 @@ def self.update_noop(name, options) puts "Using no-op. Files in #{name} may be changed but will not be committed." repo = ::Git.open("#{options[:project_root]}/#{name}") + options[:branch] ||= (repo.current_branch || 'master') repo.branch(options[:branch]).checkout puts 'Files changed:' From 1b2c94798bb5969953620e7468cae8ff403f427c Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Wed, 10 Jan 2018 21:42:33 -0500 Subject: [PATCH 2/6] Get the default branch (remotes/origin/HEAD) from the repo. Add some feature tests to verify the behavior. --- features/step_definitions/git_steps.rb | 23 +++++++++++++++++++++++ features/update.feature | 18 ++++++++++++++++++ lib/modulesync/git.rb | 10 ++++++++-- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/features/step_definitions/git_steps.rb b/features/step_definitions/git_steps.rb index dae51ed3..b28cdd15 100644 --- a/features/step_definitions/git_steps.rb +++ b/features/step_definitions/git_steps.rb @@ -22,3 +22,26 @@ git_base: file://#{expand_path('.')}/ CONFIG end + +Given /a remote module repository with "(.+?)" as the default branch/ do |branch| + steps %( + Given a directory named "sources" + And I run `git clone --mirror https://github.com/maestrodev/puppet-test sources/puppet-test` + And a file named "managed_modules.yml" with: + """ + --- + - puppet-test + """ + ) + write_file('modulesync.yml', <<-CONFIG) +--- + namespace: sources + git_base: file://#{expand_path('.')}/ + CONFIG + cd('sources/puppet-test') do + steps %( + And I run `git branch -M master #{branch}` + And I run `git symbolic-ref HEAD refs/heads/#{branch}` + ) + end +end diff --git a/features/update.feature b/features/update.feature index 6005f240..bd611da5 100644 --- a/features/update.feature +++ b/features/update.feature @@ -885,3 +885,21 @@ Feature: update Then the exit status should be 0 Then the output should not contain "error" Then the output should not contain "rejected" + + Scenario: Repository with a default branch other than master + Given a mocked git configuration + And a remote module repository with "develop" as the default branch + And a file named "config_defaults.yml" with: + """ + --- + Gemfile: + gem_source: https://somehost.com + """ + And a directory named "moduleroot" + And a file named "moduleroot/Gemfile.erb" with: + """ + source '<%= @configs['gem_source'] %>' + """ + When I run `msync update -m "Update Gemfile"` + Then the exit status should be 0 + Then the output should contain "Using repository's default branch: develop" diff --git a/lib/modulesync/git.rb b/lib/modulesync/git.rb index 367dd07f..1c1e60e5 100644 --- a/lib/modulesync/git.rb +++ b/lib/modulesync/git.rb @@ -18,10 +18,16 @@ def self.remote_branch_differ?(repo, local_branch, remote_branch) repo.diff("#{local_branch}..origin/#{remote_branch}").any? end + def self.default_branch(repo) + symbolic_ref = repo.branches.find { |b| b.full =~ %r{remotes/origin/HEAD} } + return unless symbolic_ref + %r{remotes/origin/HEAD\s+->\s+origin/(?.+?)$}.match(symbolic_ref.full)[:branch] + end + def self.switch_branch(repo, branch) unless branch - puts "Using repository's default branch" - return + branch = default_branch(repo) + puts "Using repository's default branch: #{branch}" end return if repo.current_branch == branch From ad29f4cb54ab521dd29c20b69371cb838372530a Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Fri, 12 Jan 2018 07:06:06 -0500 Subject: [PATCH 3/6] No matter what I tried, rubocop complained about this line. Disable it for now. --- features/step_definitions/git_steps.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/step_definitions/git_steps.rb b/features/step_definitions/git_steps.rb index b28cdd15..7fb92615 100644 --- a/features/step_definitions/git_steps.rb +++ b/features/step_definitions/git_steps.rb @@ -23,7 +23,7 @@ CONFIG end -Given /a remote module repository with "(.+?)" as the default branch/ do |branch| +Given /a remote module repository with "(.+?)" as the default branch/ do |branch| # rubocop:disable Lint/AmbiguousRegexpLiteral steps %( Given a directory named "sources" And I run `git clone --mirror https://github.com/maestrodev/puppet-test sources/puppet-test` From 038fb7c3a5d6fd8a6e06bfd7922f51e5f82e9a02 Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Sat, 13 Jan 2018 08:45:38 -0500 Subject: [PATCH 4/6] Disable Metrics/ModuleLength on ModuleSync::Git. --- lib/modulesync/git.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/modulesync/git.rb b/lib/modulesync/git.rb index 1c1e60e5..d062f8c4 100644 --- a/lib/modulesync/git.rb +++ b/lib/modulesync/git.rb @@ -2,7 +2,7 @@ require 'puppet_blacksmith' module ModuleSync - module Git + module Git # rubocop:disable Metrics/ModuleLength include Constants def self.remote_branch_exists?(repo, branch) From 88e8a283fef706921bb8394d6267995e40797b9d Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Tue, 16 Jan 2018 21:13:11 -0500 Subject: [PATCH 5/6] Refactor to not modify the options hash. Extract the branch selection and checkout to a helper method so Rubocop does not complain about cyclomatic complexity or the ABC metric on ModuleSync::Git::update. --- lib/modulesync/git.rb | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/modulesync/git.rb b/lib/modulesync/git.rb index d062f8c4..ebccae10 100644 --- a/lib/modulesync/git.rb +++ b/lib/modulesync/git.rb @@ -105,13 +105,18 @@ def self.tag(repo, version, tag_pattern) repo.push('origin', tag) end + def self.checkout_branch(repo, branch) + selected_branch = branch || repo.current_branch || 'master' + repo.branch(selected_branch).checkout + selected_branch + end + # Git add/rm, git commit, git push def self.update(name, files, options) module_root = "#{options[:project_root]}/#{name}" message = options[:message] repo = ::Git.open(module_root) - options[:branch] ||= (repo.current_branch || 'master') - repo.branch(options[:branch]).checkout + branch = checkout_branch(repo, options[:branch]) files.each do |file| if repo.status.deleted.include?(file) repo.remove(file) @@ -130,11 +135,11 @@ def self.update(name, files, options) end repo.commit(message, opts_commit) if options[:remote_branch] - if remote_branch_differ?(repo, options[:branch], options[:remote_branch]) - repo.push('origin', "#{options[:branch]}:#{options[:remote_branch]}", opts_push) + if remote_branch_differ?(repo, branch, options[:remote_branch]) + repo.push('origin', "#{branch}:#{options[:remote_branch]}", opts_push) end else - repo.push('origin', options[:branch], opts_push) + repo.push('origin', branch, opts_push) end # Only bump/tag if pushing didn't fail (i.e. there were changes) m = Blacksmith::Modulefile.new("#{module_root}/metadata.json") @@ -165,8 +170,7 @@ def self.update_noop(name, options) puts "Using no-op. Files in #{name} may be changed but will not be committed." repo = ::Git.open("#{options[:project_root]}/#{name}") - options[:branch] ||= (repo.current_branch || 'master') - repo.branch(options[:branch]).checkout + checkout_branch(repo, options[:branch]) puts 'Files changed:' repo.diff('HEAD', '--').each do |diff| From 2fe4137b75867b5762f9ee6ed71dab333f030cda Mon Sep 17 00:00:00 2001 From: Sean Edge Date: Wed, 17 Jan 2018 06:01:10 -0500 Subject: [PATCH 6/6] Make helper method private and improve documentation on CLI option. --- lib/modulesync/cli.rb | 2 +- lib/modulesync/git.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/modulesync/cli.rb b/lib/modulesync/cli.rb index 60c79623..cf5ab907 100644 --- a/lib/modulesync/cli.rb +++ b/lib/modulesync/cli.rb @@ -53,7 +53,7 @@ class Base < Thor :desc => 'A regular expression to skip repositories.' class_option :branch, :aliases => '-b', - :desc => 'Branch name to make the changes in. Defaults to the default branch of the upstream repository.', + :desc => 'Branch name to make the changes in. Defaults to the default branch of the upstream repository, but falls back to "master".', :default => CLI.defaults[:branch] desc 'update', 'Update the modules in managed_modules.yml' diff --git a/lib/modulesync/git.rb b/lib/modulesync/git.rb index ebccae10..cc064657 100644 --- a/lib/modulesync/git.rb +++ b/lib/modulesync/git.rb @@ -110,6 +110,7 @@ def self.checkout_branch(repo, branch) repo.branch(selected_branch).checkout selected_branch end + private_class_method :checkout_branch # Git add/rm, git commit, git push def self.update(name, files, options)