Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix an issue with stash #623

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix an issue with stash #623

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c17b111
Use git stash 'push' rather than deprecated git stash 'save'
adamretter Sep 18, 2023
6549317
Fix a bug when 'keep_local_changes => true' is set 'git stash push' …
adamretter Sep 18, 2023
ad9f4d5
Address code-review comments
adamretter Sep 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions lib/puppet/provider/vcsrepo/git.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,16 +423,17 @@ def commits?
# handle upstream branch changes
# @!visibility private
def checkout(revision = @resource.value(:revision))
has_changes = uncommitted_changes?
keep_local_changes = @resource.value(:keep_local_changes)
stash if keep_local_changes == :true
stash if has_changes == :true && keep_local_changes == :true
if !local_branch_revision?(revision) && remote_branch_revision?(revision)
# non-locally existant branches (perhaps switching to a branch that has never been checked out)
at_path { git_with_identity('checkout', '--force', '-b', revision, '--track', "#{@resource.value(:remote)}/#{revision}") }
else
# tags, locally existant branches (perhaps outdated), and shas
at_path { git_with_identity('checkout', '--force', revision) }
end
unstash if keep_local_changes == :true
unstash if has_changes == :true && keep_local_changes == :true
end

# @!visibility private
Expand Down Expand Up @@ -503,9 +504,16 @@ def set_excludes
end
end

# @!visibility private
def uncommitted_changes?
at_path do
git_with_identity('status', '--porcelain').empty?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change the sequence of commands run, you need to adjust CI to take this into account (see failed CI at the bottom of the "Conversation" tab).

Need guidance? Read on 😃

When reading the output from RSpec, you can identify 3 sections:

  1. The output of the various tests with some details on failures;
  2. A list of all failures details with backtraces;
  3. A list of failed tests.

3 is the most handy to locate the tests you want to adjust (direct view of files and lines), while 1 & 2 tell you what is wrong.

If you consider the first failure in the "third section", you see that the error occurred in this test:

puppetlabs-vcsrepo/spec/unit/puppet/provider/vcsrepo/git_spec.rb

Lines 34 to 44 in 2b05c3a

it "executes 'git clone' and 'git checkout -b'" do
resource[:revision] = 'only/remote'
expect(Dir).to receive(:chdir).with('/').once.and_yield
expect(Dir).to receive(:chdir).with('/tmp/test').at_least(:once).and_yield
expect(provider).to receive(:exec_git).with('clone', resource.value(:source), resource.value(:path))
expect(provider).to receive(:update_submodules)
expect(provider).to receive(:update_remote_url).with('origin', resource.value(:source)).and_return false
expect(provider).to receive(:exec_git).with('branch', '--no-color', '-a').and_return(branch_a_list(resource.value(:revision)))
expect(provider).to receive(:exec_git).with('checkout', '--force', resource.value(:revision))
provider.create
end

If you scroll up in the second section, you see the actual error:

  1) Puppet::Type::Vcsrepo::ProviderGit when with an ensure of present when with an ensure of present - with a revision that is a remote branch executes 'git clone' and 'git checkout -b'
     Failure/Error: exec_git(*args)

       Vcsrepo[test](provider=git) received :exec_git with unexpected arguments
         expected: ("branch", "--no-color", "-a")
              got: ("status", "--porcelain")
     # ./lib/puppet/provider/vcsrepo/git.rb:691:in `git_with_identity'
     # ./lib/puppet/provider/vcsrepo/git.rb:510:in `block in uncommitted_changes?'
     # ./lib/puppet/provider/vcsrepo.rb:53:in `block in at_path'
     # ./lib/puppet/provider/vcsrepo.rb:52:in `at_path'
     # ./lib/puppet/provider/vcsrepo/git.rb:509:in `uncommitted_changes?'
     # ./lib/puppet/provider/vcsrepo/git.rb:426:in `checkout'
     # ./lib/puppet/provider/vcsrepo/git.rb:22:in `create'
     # ./spec/unit/puppet/provider/vcsrepo/git_spec.rb:43:in `block (4 levels) in <top (required)>'
     # ./vendor/bundle/ruby/2.7.0/bin/rspec:23:in `load'
     # ./vendor/bundle/ruby/2.7.0/bin/rspec:23:in `<top (required)>'
     # /opt/hostedtoolcache/Ruby/2.7.8/x64/bin/bundle:23:in `load'
     # /opt/hostedtoolcache/Ruby/2.7.8/x64/bin/bundle:23:in `<main>'

In this case, adding a line like the following one between lines 40 and 41 should fix the issue:

   expect(provider).to receive(:exec_git).with('status', '--porcelain').and_return('')

Thanks!

end
end

# @!visibility private
def stash
at_path { git_with_identity('stash', 'save') }
at_path { git_with_identity('stash', 'push') }
end

# @!visibility private
Expand Down