Skip to content

submodule cloning may not be important to the job, ignore errors #6172

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

Merged
jakecoffman merged 1 commit into from
Nov 28, 2022
Merged

submodule cloning may not be important to the job, ignore errors #6172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 25 additions & 7 deletions common/lib/dependabot/file_fetchers/base.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class Base
Dependabot::Clients::CodeCommit::NotFound
].freeze

GIT_SUBMODULE_ERROR_REGEX =
/^fatal: unable to access '(?<url>.*)': The requested URL returned error: (?<code>\d+)$/

def self.required_files_in?(_filename_array)
raise NotImplementedError
end
Expand Down Expand Up @@ -592,19 +595,34 @@ def _clone_repo_contents(target_directory:)
" --no-recurse-submodules"
end
clone_options << " --branch #{source.branch} --single-branch" if source.branch
SharedHelpers.run_shell_command(
<<~CMD
git clone #{clone_options.string} #{source.url} #{path}
CMD
)

submodule_cloning_failed = false
begin
SharedHelpers.run_shell_command(
<<~CMD
git clone #{clone_options.string} #{source.url} #{path}
CMD
)
rescue SharedHelpers::HelperSubprocessFailed => e
raise unless e.message.match?(GIT_SUBMODULE_ERROR_REGEX) && e.message.include?("submodule")

submodule_cloning_failed = true
match = e.message.match(GIT_SUBMODULE_ERROR_REGEX)
url = match.named_captures["url"]
code = match.named_captures["code"]

# Submodules might be in the repo but unrelated to dependencies,
# so ignoring this error to try the update anyway since the base repo exists.
Dependabot.logger.error("Cloning of submodule failed: #{url} error: #{code}")
Copy link
Copy Markdown
Contributor

@landongrindheim landongrindheim Nov 28, 2022

Choose a reason for hiding this comment

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

👌 I can picture this being really helpful 😄

end

if source.commit
# This code will only be called for testing. Production will never pass a commit
# since Dependabot always wants to use the latest commit on a branch.
Dir.chdir(path) do
fetch_options = StringIO.new
fetch_options << "--depth 1"
fetch_options << if recurse_submodules_when_cloning?
fetch_options << if recurse_submodules_when_cloning? && !submodule_cloning_failed
" --recurse-submodules=on-demand"
else
" --no-recurse-submodules"
Expand All @@ -614,7 +632,7 @@ def _clone_repo_contents(target_directory:)

reset_options = StringIO.new
reset_options << "--hard"
reset_options << if recurse_submodules_when_cloning?
reset_options << if recurse_submodules_when_cloning? && !submodule_cloning_failed
" --recurse-submodules"
else
" --no-recurse-submodules"
Expand Down
11 changes: 11 additions & 0 deletions common/spec/dependabot/file_fetchers/base_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1564,6 +1564,17 @@ def fetch_files
expect { subject }.to raise_error(Dependabot::BranchNotFound)
end
end

context "when the submodule can't be reached" do
let(:repo) do
"dependabot-fixtures/go-modules-app-with-inaccessible-submodules"
end

it "does not raise an error" do
clone_repo_contents
expect(`ls #{repo_contents_path}`).to include("README")
end
end
end
end

Expand Down