Skip to content

Allow file fetchers to opt into loading git submodules #5982

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
bdragon merged 4 commits into from
Oct 27, 2022
Merged

Allow file fetchers to opt into loading git submodules #5982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0b1f097
Reorganize method definition order for clarity
bdragon Oct 24, 2022
41bac21
Add #recurse_submodules_when_cloning? to control cloning behavior
bdragon Oct 26, 2022
2f8a624
Enable #recurse_submodules_when_cloning? for go_modules file fetcher
bdragon Oct 26, 2022
de50423
Merge branch 'main' into bdragon/clone-recurse-submodules
bdragon Oct 27, 2022
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
220 changes: 129 additions & 91 deletions common/lib/dependabot/file_fetchers/base.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true

require "stringio"
require "dependabot/config"
require "dependabot/dependency_file"
require "dependabot/source"
Expand Down Expand Up @@ -173,6 +174,97 @@ def repo_contents(dir: ".", ignore_base_directory: false,
end
end

def cloned_commit
return if repo_contents_path.nil? || !File.directory?(File.join(repo_contents_path, ".git"))

SharedHelpers.with_git_configured(credentials: credentials) do
Dir.chdir(repo_contents_path) do
return SharedHelpers.run_shell_command("git rev-parse HEAD")&.strip
end
end
end

def default_branch_for_repo
@default_branch_for_repo ||= client_for_provider.
fetch_default_branch(repo)
rescue *CLIENT_NOT_FOUND_ERRORS
raise Dependabot::RepoNotFound, source
end

def update_linked_paths(repo, path, commit, github_response)
case github_response.type
when "submodule"
sub_source = Source.from_url(github_response.submodule_git_url)
return unless sub_source

@linked_paths[path] = {
repo: sub_source.repo,
provider: sub_source.provider,
commit: github_response.sha,
path: "/"
}
when "symlink"
updated_path = File.join(File.dirname(path), github_response.target)
@linked_paths[path] = {
repo: repo,
provider: "github",
commit: commit,
path: Pathname.new(updated_path).cleanpath.to_path
}
end
end

def recurse_submodules_when_cloning?
false
end

def client_for_provider
case source.provider
when "github" then github_client
when "gitlab" then gitlab_client
when "azure" then azure_client
when "bitbucket" then bitbucket_client
when "codecommit" then codecommit_client
else raise "Unsupported provider '#{source.provider}'."
end
end

def github_client
@github_client ||=
Dependabot::Clients::GithubWithRetries.for_source(
source: source,
credentials: credentials
)
end

def gitlab_client
@gitlab_client ||=
Dependabot::Clients::GitlabWithRetries.for_source(
source: source,
credentials: credentials
)
end

def azure_client
@azure_client ||=
Dependabot::Clients::Azure.
for_source(source: source, credentials: credentials)
end

def bitbucket_client
# TODO: When self-hosted Bitbucket is supported this should use
# `Bitbucket.for_source`
@bitbucket_client ||=
Dependabot::Clients::BitbucketWithRetries.
for_bitbucket_dot_org(credentials: credentials)
end

def codecommit_client
@codecommit_client ||=
Dependabot::Clients::CodeCommit.
for_source(source: source, credentials: credentials)
end

#################################################
# INTERNAL METHODS (not for use by sub-classes) #
#################################################
Expand Down Expand Up @@ -259,29 +351,6 @@ def _cloned_repo_contents(relative_path)
end
end

def update_linked_paths(repo, path, commit, github_response)
case github_response.type
when "submodule"
sub_source = Source.from_url(github_response.submodule_git_url)
return unless sub_source

@linked_paths[path] = {
repo: sub_source.repo,
provider: sub_source.provider,
commit: github_response.sha,
path: "/"
}
when "symlink"
updated_path = File.join(File.dirname(path), github_response.target)
@linked_paths[path] = {
repo: repo,
provider: "github",
commit: commit,
path: Pathname.new(updated_path).cleanpath.to_path
}
end
end

def _build_github_file_struct(file)
OpenStruct.new(
name: file.name,
Expand Down Expand Up @@ -478,23 +547,6 @@ def _fetch_file_content_from_github(path, repo, commit)
end
# rubocop:enable Metrics/AbcSize

def cloned_commit
return if repo_contents_path.nil? || !File.directory?(File.join(repo_contents_path, ".git"))

SharedHelpers.with_git_configured(credentials: credentials) do
Dir.chdir(repo_contents_path) do
return SharedHelpers.run_shell_command("git rev-parse HEAD")&.strip
end
end
end

def default_branch_for_repo
@default_branch_for_repo ||= client_for_provider.
fetch_default_branch(repo)
rescue *CLIENT_NOT_FOUND_ERRORS
raise Dependabot::RepoNotFound, source
end

# Update the @linked_paths hash by exploiting a side-effect of
# recursively calling `repo_contents` for each directory up the tree
# until a submodule or symlink is found
Expand All @@ -519,6 +571,10 @@ def _linked_dir_for(path)
max_by(&:length)
end

# rubocop:disable Metrics/AbcSize
# rubocop:disable Metrics/MethodLength
# rubocop:disable Metrics/PerceivedComplexity
# rubocop:disable Metrics/BlockLength
def _clone_repo_contents(target_directory:)
SharedHelpers.with_git_configured(credentials: credentials) do
path = target_directory || File.join("tmp", source.repo)
Expand All @@ -527,72 +583,54 @@ def _clone_repo_contents(target_directory:)
return path if Dir.exist?(File.join(path, ".git"))

FileUtils.mkdir_p(path)
br_opt = " --branch #{source.branch} --single-branch" if source.branch

clone_options = StringIO.new
clone_options << "--no-tags --depth 1"
clone_options << if recurse_submodules_when_cloning?
" --recurse-submodules --shallow-submodules"
else
" --no-recurse-submodules"
end
clone_options << " --branch #{source.branch} --single-branch" if source.branch
SharedHelpers.run_shell_command(
<<~CMD
git clone --no-tags --no-recurse-submodules --depth 1#{br_opt} #{source.url} #{path}
git clone #{clone_options.string} #{source.url} #{path}
CMD
)

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?
" --recurse-submodules=on-demand"
else
" --no-recurse-submodules"
end
# Need to fetch the commit due to the --depth 1 above.
SharedHelpers.run_shell_command("git fetch --depth 1 origin #{source.commit}")
SharedHelpers.run_shell_command("git fetch #{fetch_options.string} origin #{source.commit}")

reset_options = StringIO.new
reset_options << "--hard"
reset_options << if recurse_submodules_when_cloning?
" --recurse-submodules"
else
" --no-recurse-submodules"
end
# Set HEAD to this commit so later calls so git reset HEAD will work.
SharedHelpers.run_shell_command("git reset --hard #{source.commit}")
SharedHelpers.run_shell_command("git reset #{reset_options.string} #{source.commit}")
end
end
path
end
end

def client_for_provider
case source.provider
when "github" then github_client
when "gitlab" then gitlab_client
when "azure" then azure_client
when "bitbucket" then bitbucket_client
when "codecommit" then codecommit_client
else raise "Unsupported provider '#{source.provider}'."
path
end
end

def github_client
@github_client ||=
Dependabot::Clients::GithubWithRetries.for_source(
source: source,
credentials: credentials
)
end

def gitlab_client
@gitlab_client ||=
Dependabot::Clients::GitlabWithRetries.for_source(
source: source,
credentials: credentials
)
end

def azure_client
@azure_client ||=
Dependabot::Clients::Azure.
for_source(source: source, credentials: credentials)
end

def bitbucket_client
# TODO: When self-hosted Bitbucket is supported this should use
# `Bitbucket.for_source`
@bitbucket_client ||=
Dependabot::Clients::BitbucketWithRetries.
for_bitbucket_dot_org(credentials: credentials)
end

def codecommit_client
@codecommit_client ||=
Dependabot::Clients::CodeCommit.
for_source(source: source, credentials: credentials)
end
# rubocop:enable Metrics/AbcSize
# rubocop:enable Metrics/MethodLength
# rubocop:enable Metrics/PerceivedComplexity
# rubocop:enable Metrics/BlockLength
end
end
end
Expand Down
94 changes: 94 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 @@ -7,6 +7,7 @@
require "dependabot/source"
require "dependabot/file_fetchers/base"
require "dependabot/clients/codecommit"
require "dependabot/shared_helpers"

RSpec.describe Dependabot::FileFetchers::Base do
let(:source) do
Expand Down Expand Up @@ -1565,4 +1566,97 @@ def fetch_files
end
end
end

context "with submodules" do
let(:repo) { "dependabot-fixtures/go-modules-app-with-git-submodules" }
let(:repo_contents_path) { Dir.mktmpdir }
let(:submodule_contents_path) { File.join(repo_contents_path, "examplelib") }

before do
allow(Dependabot::SharedHelpers).
to receive(:run_shell_command).and_call_original
end

after { FileUtils.rm_rf(repo_contents_path) }

describe "#clone_repo_contents" do
it "does not clone submodules by default" do
file_fetcher_instance.clone_repo_contents

expect(Dependabot::SharedHelpers).
to have_received(:run_shell_command).with(
/\Agit clone .* --no-recurse-submodules/
)
expect(`ls -1 #{submodule_contents_path}`.split).to_not include("go.mod")
end

context "with a source commit" do
let(:source_commit) { "5c7e92a4860382fd31336872f0fe79a848669c4d" }

it "does not fetch/reset submodules by default" do
file_fetcher_instance.clone_repo_contents

expect(Dependabot::SharedHelpers).
to have_received(:run_shell_command).with(
/\Agit fetch .* --no-recurse-submodules/
)
expect(Dependabot::SharedHelpers).
to have_received(:run_shell_command).with(
/\Agit reset .* --no-recurse-submodules/
)
end
end

context "when #recurse_submodules_when_cloning? returns true" do
let(:child_class) do
Class.new(described_class) do
def self.required_files_in?(filenames)
filenames.include?("go.mod")
end

def self.required_files_message
"Repo must contain a go.mod."
end

private

def fetch_files
[fetch_file_from_host("go.mod")]
end

def recurse_submodules_when_cloning?
true
end
end
end

it "clones submodules" do
file_fetcher_instance.clone_repo_contents

expect(Dependabot::SharedHelpers).
to have_received(:run_shell_command).with(
/\Agit clone .* --recurse-submodules --shallow-submodules/
)
expect(`ls -1 #{submodule_contents_path}`.split).to include("go.mod")
end

context "with a source commit" do
let(:source_commit) { "5c7e92a4860382fd31336872f0fe79a848669c4d" }

it "fetches/resets submodules if necessary" do
file_fetcher_instance.clone_repo_contents

expect(Dependabot::SharedHelpers).
to have_received(:run_shell_command).with(
/\Agit fetch .* --recurse-submodules=on-demand/
)
expect(Dependabot::SharedHelpers).
to have_received(:run_shell_command).with(
/\Agit reset .* --recurse-submodules/
)
end
end
end
end
end
end
4 changes: 4 additions & 0 deletions go_modules/lib/dependabot/go_modules/file_fetcher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ def go_mod
def go_sum
@go_sum ||= fetch_file_if_present("go.sum")
end

def recurse_submodules_when_cloning?
true
Copy link
Copy Markdown
Member

@Nishnha Nishnha Nov 16, 2022

Choose a reason for hiding this comment

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

Something we can try here to mitigate the issue @thepwagner brought up is only install submodules if the initial dependency resolution step failed to run

We do something of that sorts for Terraform modules in

dependabot-core/terraform/lib/dependabot/terraform/file_updater.rb

Lines 242 to 253 in 95df3a4

rescue SharedHelpers::HelperSubprocessFailed => e
if @retrying_lock && e.message.match?(MODULE_NOT_INSTALLED_ERROR)
mod = e.message.match(MODULE_NOT_INSTALLED_ERROR).named_captures.fetch("mod")
raise Dependabot::DependencyFileNotResolvable, "Attempt to install module #{mod} failed"
end
raise if @retrying_lock || !e.message.include?("terraform init")
# NOTE: Modules need to be installed before terraform can update the lockfile
@retrying_lock = true
run_terraform_init
retry
end

Copy link
Copy Markdown
Member

@jakecoffman jakecoffman Nov 16, 2022

Choose a reason for hiding this comment

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

Another option might be we leave the submodule clone by default, but ignore any errors to attempt the update anyway? That way if it didn't matter then we still do the update.

end
end
end
end
Expand Down
Loading