From 0cdebde199ca930a2367747fead799a76e38b5a6 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Wed, 20 Apr 2022 20:57:40 +0100 Subject: [PATCH 1/6] Detect includeBuild in Gradle SettingsFileParser Detect includeBuild statements used to configure composite builds in the Gradle SettingsFileParser. https://docs.gradle.org/current/userguide/composite_builds.html https://docs.gradle.org/current/javadoc/org/gradle/api/initialization/Settings.html#includeBuild-java.lang.Object https://docs.gradle.org/current/javadoc/org/gradle/plugin/management/PluginManagementSpec.html#includeBuild-java.lang.String- --- .../file_fetcher/settings_file_parser.rb | 9 +++ .../file_fetcher/settings_file_parser_spec.rb | 72 ++++++++++++++++--- .../dependabot/gradle/file_fetcher_spec.rb | 2 +- .../settings_files/call_style_settings.gradle | 6 ++ .../settings_files/comment_settings.gradle | 8 +++ .../composite_build_settings.gradle | 6 ++ .../composite_build_simple_settings.gradle | 2 + .../settings_files/settings.gradle.kts | 7 ++ 8 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 gradle/spec/fixtures/settings_files/composite_build_settings.gradle create mode 100644 gradle/spec/fixtures/settings_files/composite_build_simple_settings.gradle diff --git a/gradle/lib/dependabot/gradle/file_fetcher/settings_file_parser.rb b/gradle/lib/dependabot/gradle/file_fetcher/settings_file_parser.rb index dc37df4e92c..1e247c1644f 100644 --- a/gradle/lib/dependabot/gradle/file_fetcher/settings_file_parser.rb +++ b/gradle/lib/dependabot/gradle/file_fetcher/settings_file_parser.rb @@ -10,6 +10,15 @@ def initialize(settings_file:) @settings_file = settings_file end + def included_build_paths + paths = [] + comment_free_content.scan(function_regex("includeBuild")) do + arg = Regexp.last_match.named_captures.fetch("args") + paths << arg.gsub(/["']/, "").strip + end + paths.uniq + end + def subproject_paths subprojects = [] diff --git a/gradle/spec/dependabot/gradle/file_fetcher/settings_file_parser_spec.rb b/gradle/spec/dependabot/gradle/file_fetcher/settings_file_parser_spec.rb index 3f8d070797e..7a1ebf5e1d2 100644 --- a/gradle/spec/dependabot/gradle/file_fetcher/settings_file_parser_spec.rb +++ b/gradle/spec/dependabot/gradle/file_fetcher/settings_file_parser_spec.rb @@ -6,13 +6,13 @@ RSpec.describe Dependabot::Gradle::FileFetcher::SettingsFileParser do let(:finder) { described_class.new(settings_file: settings_file) } - let(:settings_file) do Dependabot::DependencyFile.new( - name: "settings.gradle", + name: settings_file_name, content: fixture("settings_files", fixture_name) - ) + ) end + let(:settings_file_name) { "settings.gradle" } let(:fixture_name) { "simple_settings.gradle" } describe "#subproject_paths" do @@ -37,12 +37,7 @@ end context "when kotlin" do - let(:settings_file) do - Dependabot::DependencyFile.new( - name: "settings.gradle.kts", - content: fixture("settings_files", fixture_name) - ) - end + let(:settings_file_name) { "settings.gradle.kts" } let(:fixture_name) { "settings.gradle.kts" } it "includes the additional declarations" do @@ -85,4 +80,63 @@ end end end + + describe "#included_build_paths" do + subject(:included_build_paths) { finder.included_build_paths } + + context "when there are no included build declarations" do + let(:fixture_name) { "simple_settings.gradle" } + + it "includes no declaration" do + expect(included_build_paths).to match_array([]) + end + end + + context "with single included build" do + let(:fixture_name) { "composite_build_simple_settings.gradle" } + + it "includes the declaration" do + expect(included_build_paths).to match_array(%w(./included)) + end + end + + context "with multiple included builds" do + let(:fixture_name) { "composite_build_settings.gradle" } + + it "includes the additional declarations" do + expect(included_build_paths).to match_array( + %w(./plugins/lint-plugins ./plugins/settings-plugins ./publishing) + ) + end + end + + context "with various call styles" do + let(:fixture_name) { "call_style_settings.gradle" } + + it "includes all declarations" do + expect(included_build_paths).to match_array( + %w(without_space with_space implicit implicit_with_many_spaces ./standard-path) + ) + end + end + + context "with commented out included build declarations" do + let(:fixture_name) { "comment_settings.gradle" } + + it "includes only uncommented declarations" do + expect(included_build_paths).to match_array(%w(./included)) + end + end + + # TODO context "with commented out included build declarations" + + context "when kotlin" do + let(:settings_file_name) { "settings.gradle.kts" } + let(:fixture_name) { "settings.gradle.kts" } + + it "includes the additional declarations" do + expect(included_build_paths).to match_array(%w(./settings-plugins ./project-plugins)) + end + end + end end diff --git a/gradle/spec/dependabot/gradle/file_fetcher_spec.rb b/gradle/spec/dependabot/gradle/file_fetcher_spec.rb index 3e0a664ca23..703b519972e 100644 --- a/gradle/spec/dependabot/gradle/file_fetcher_spec.rb +++ b/gradle/spec/dependabot/gradle/file_fetcher_spec.rb @@ -65,7 +65,7 @@ def stub_content_request(path, fixture) to match_array(%w(build.gradle settings.gradle app/build.gradle)) end - context "when the subproject can't fe found" do + context "when the subproject can't be found" do before do stub_request(:get, File.join(url, "app/build.gradle?ref=sha")). with(headers: { "Authorization" => "token token" }). diff --git a/gradle/spec/fixtures/settings_files/call_style_settings.gradle b/gradle/spec/fixtures/settings_files/call_style_settings.gradle index 1d92dc84834..06b15b40553 100644 --- a/gradle/spec/fixtures/settings_files/call_style_settings.gradle +++ b/gradle/spec/fixtures/settings_files/call_style_settings.gradle @@ -2,3 +2,9 @@ include(':function_without_space') include (':function_with_space') include ':implicit' include ":implicit_with_many_spaces" + +includeBuild('without_space') +includeBuild ('with_space') +includeBuild 'implicit' +includeBuild "implicit_with_many_spaces" +includeBuild './standard-path' diff --git a/gradle/spec/fixtures/settings_files/comment_settings.gradle b/gradle/spec/fixtures/settings_files/comment_settings.gradle index ff1470a1b4b..e1055b5039e 100644 --- a/gradle/spec/fixtures/settings_files/comment_settings.gradle +++ b/gradle/spec/fixtures/settings_files/comment_settings.gradle @@ -5,3 +5,11 @@ // include ':comment' include ':app' + +/* + * includeBuild './block_comment' + */ + +// includeBuild('./comment') + +includeBuild './included' diff --git a/gradle/spec/fixtures/settings_files/composite_build_settings.gradle b/gradle/spec/fixtures/settings_files/composite_build_settings.gradle new file mode 100644 index 00000000000..2a7656120d2 --- /dev/null +++ b/gradle/spec/fixtures/settings_files/composite_build_settings.gradle @@ -0,0 +1,6 @@ +pluginManagement { + includeBuild './plugins/settings-plugins' +} + +includeBuild './plugins/lint-plugins' +includeBuild './publishing' diff --git a/gradle/spec/fixtures/settings_files/composite_build_simple_settings.gradle b/gradle/spec/fixtures/settings_files/composite_build_simple_settings.gradle new file mode 100644 index 00000000000..8912db0cbd0 --- /dev/null +++ b/gradle/spec/fixtures/settings_files/composite_build_simple_settings.gradle @@ -0,0 +1,2 @@ +include ':app' +includeBuild './included' diff --git a/gradle/spec/fixtures/settings_files/settings.gradle.kts b/gradle/spec/fixtures/settings_files/settings.gradle.kts index 15a801b10aa..489940e3ab1 100644 --- a/gradle/spec/fixtures/settings_files/settings.gradle.kts +++ b/gradle/spec/fixtures/settings_files/settings.gradle.kts @@ -1 +1,8 @@ +pluginManagement { + includeBuild("./settings-plugins") +} + +includeBuild("./project-plugins") + include(":app") + From e045a5028fa6f8986d5c25f5eece626623962bfd Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 21 Apr 2022 16:16:13 +0100 Subject: [PATCH 2/6] Refactor FileFetcher to support transitive search --- gradle/lib/dependabot/gradle/file_fetcher.rb | 71 ++++++++++---------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/gradle/lib/dependabot/gradle/file_fetcher.rb b/gradle/lib/dependabot/gradle/file_fetcher.rb index 437c0e70ef0..bde579023e4 100644 --- a/gradle/lib/dependabot/gradle/file_fetcher.rb +++ b/gradle/lib/dependabot/gradle/file_fetcher.rb @@ -27,37 +27,28 @@ def self.required_files_message private def fetch_files - fetched_files = [] - fetched_files << buildfile if buildfile - fetched_files << settings_file if settings_file - fetched_files += subproject_buildfiles - fetched_files += dependency_script_plugins - check_required_files_present + dir = "." + fetched_files = [buildfile(dir), settings_file(dir)].compact + fetched_files += subproject_buildfiles(dir) + fetched_files += dependency_script_plugins(dir) + check_required_files_present(fetched_files) fetched_files end - def buildfile - @buildfile ||= begin - file = supported_build_file - @buildfile_name ||= file.name if file - file - end - end - - def subproject_buildfiles - return [] unless settings_file + def subproject_buildfiles(root_dir) + return [] unless settings_file(root_dir) @subproject_buildfiles ||= begin subproject_paths = SettingsFileParser. - new(settings_file: settings_file). + new(settings_file: settings_file(root_dir)). subproject_paths subproject_paths.filter_map do |path| if @buildfile_name fetch_file_from_host(File.join(path, @buildfile_name)) else - supported_file(SUPPORTED_BUILD_FILE_NAMES.map { |f| File.join(path, f) }) + buildfile(path) end rescue Dependabot::DependencyFileNotFound # Gradle itself doesn't worry about missing subprojects, so we don't @@ -67,11 +58,11 @@ def subproject_buildfiles end # rubocop:disable Metrics/PerceivedComplexity - def dependency_script_plugins - return [] unless buildfile + def dependency_script_plugins(root_dir) + return [] unless buildfile(root_dir) dependency_plugin_paths = - FileParser.find_include_names(buildfile). + FileParser.find_include_names(buildfile(root_dir)). reject { |path| path.include?("://") }. reject { |path| !path.include?("/") && path.split(".").count > 2 }. select { |filename| filename.include?("dependencies") }. @@ -89,8 +80,8 @@ def dependency_script_plugins end # rubocop:enable Metrics/PerceivedComplexity - def check_required_files_present - return if buildfile || (subproject_buildfiles && !subproject_buildfiles.empty?) + def check_required_files_present(files) + return if files.any? path = Pathname.new(File.join(directory, "build.gradle")).cleanpath.to_path path += "(.kts)?" @@ -104,24 +95,36 @@ def file_exists_in_submodule?(path) false end - def settings_file - @settings_file ||= supported_settings_file + def buildfile(dir) + file = first_present_file(dir, SUPPORTED_BUILD_FILE_NAMES) + @buildfile_name = file.name if file + file + end + + def settings_file(dir) + first_present_file(dir, SUPPORTED_SETTINGS_FILE_NAMES) end - def supported_build_file - supported_file(SUPPORTED_BUILD_FILE_NAMES) + def first_present_file(dir, supported_names) + paths = supported_names.map { |name| File.join(dir, name) } + paths.each do |path| + return cache[path] if cache.include? path + end + fetch_first_present_file(paths) end - def supported_settings_file - supported_file(SUPPORTED_SETTINGS_FILE_NAMES) + def cache + @cached_files ||= Hash.new end - def supported_file(supported_file_names) - supported_file_names.each do |supported_file_name| - file = fetch_file_if_present(supported_file_name) - return file if file + def fetch_first_present_file(paths) + paths.each do |path| + file = fetch_file_if_present(path) + if file + cache[path] = file + return file + end end - nil end end From 72d4a3495c6f5c87ed6245575d76469151bd06c6 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Wed, 20 Apr 2022 22:13:38 +0100 Subject: [PATCH 3/6] Fetch included builds in Gradle FileFetcher Fetch build files recursively for included builds in the Gradle FileFetcher. --- gradle/lib/dependabot/gradle/file_fetcher.rb | 88 ++++++++------ .../dependabot/gradle/file_fetcher_spec.rb | 115 ++++++++++++++++++ ...ntents_java_settings_1_included_build.json | 18 +++ ...tents_java_settings_2_included_builds.json | 18 +++ 4 files changed, 201 insertions(+), 38 deletions(-) create mode 100644 gradle/spec/fixtures/github/contents_java_settings_1_included_build.json create mode 100644 gradle/spec/fixtures/github/contents_java_settings_2_included_builds.json diff --git a/gradle/lib/dependabot/gradle/file_fetcher.rb b/gradle/lib/dependabot/gradle/file_fetcher.rb index bde579023e4..6800064d642 100644 --- a/gradle/lib/dependabot/gradle/file_fetcher.rb +++ b/gradle/lib/dependabot/gradle/file_fetcher.rb @@ -27,34 +27,46 @@ def self.required_files_message private def fetch_files - dir = "." - fetched_files = [buildfile(dir), settings_file(dir)].compact - fetched_files += subproject_buildfiles(dir) - fetched_files += dependency_script_plugins(dir) - check_required_files_present(fetched_files) - fetched_files + files = all_buildfiles_in_build(".") + check_required_files_present(files) + files + end + + def all_buildfiles_in_build(root_dir) + files = [buildfile(root_dir), settings_file(root_dir)].compact + files += subproject_buildfiles(root_dir) + files += dependency_script_plugins(root_dir) + files += included_builds(root_dir). + flat_map { |dir| all_buildfiles_in_build(dir) } + end + + def included_builds(root_dir) + return [] unless settings_file(root_dir) + + SettingsFileParser. + new(settings_file: settings_file(root_dir)). + included_build_paths. + map { |p| File.join(root_dir, p) } end def subproject_buildfiles(root_dir) return [] unless settings_file(root_dir) - @subproject_buildfiles ||= begin - subproject_paths = - SettingsFileParser. - new(settings_file: settings_file(root_dir)). - subproject_paths - - subproject_paths.filter_map do |path| - if @buildfile_name - fetch_file_from_host(File.join(path, @buildfile_name)) - else - buildfile(path) - end - rescue Dependabot::DependencyFileNotFound - # Gradle itself doesn't worry about missing subprojects, so we don't - nil + subproject_paths = + SettingsFileParser. + new(settings_file: settings_file(root_dir)). + subproject_paths + + subproject_paths.map do |path| + if @buildfile_name + fetch_file_from_host(File.join(root_dir, path, @buildfile_name)) + else + buildfile(File.join(root_dir, path)) end - end + rescue Dependabot::DependencyFileNotFound + # Gradle itself doesn't worry about missing subprojects, so we don't + nil + end.compact end # rubocop:disable Metrics/PerceivedComplexity @@ -67,6 +79,7 @@ def dependency_script_plugins(root_dir) reject { |path| !path.include?("/") && path.split(".").count > 2 }. select { |filename| filename.include?("dependencies") }. map { |path| path.gsub("$rootDir", ".") }. + map { |path| File.join(root_dir, path) }. uniq dependency_plugin_paths.filter_map do |path| @@ -96,34 +109,33 @@ def file_exists_in_submodule?(path) end def buildfile(dir) - file = first_present_file(dir, SUPPORTED_BUILD_FILE_NAMES) - @buildfile_name = file.name if file + file = find_first(dir, SUPPORTED_BUILD_FILE_NAMES) || return + @buildfile_name ||= File.basename(file.name) file end def settings_file(dir) - first_present_file(dir, SUPPORTED_SETTINGS_FILE_NAMES) + find_first(dir, SUPPORTED_SETTINGS_FILE_NAMES) end - def first_present_file(dir, supported_names) - paths = supported_names.map { |name| File.join(dir, name) } - paths.each do |path| - return cache[path] if cache.include? path - end - fetch_first_present_file(paths) + def find_first(dir, supported_names) + paths = supported_names. + map { |name| File.join(dir, name) }. + each do |path| + return cached_files[path] || next + end + fetch_first_if_present(paths) end - def cache + def cached_files @cached_files ||= Hash.new end - def fetch_first_present_file(paths) + def fetch_first_if_present(paths) paths.each do |path| - file = fetch_file_if_present(path) - if file - cache[path] = file - return file - end + file = fetch_file_if_present(path) || next + cached_files[path] = file + return file end nil end diff --git a/gradle/spec/dependabot/gradle/file_fetcher_spec.rb b/gradle/spec/dependabot/gradle/file_fetcher_spec.rb index 703b519972e..8597777c1f1 100644 --- a/gradle/spec/dependabot/gradle/file_fetcher_spec.rb +++ b/gradle/spec/dependabot/gradle/file_fetcher_spec.rb @@ -80,6 +80,121 @@ def stub_content_request(path, fixture) end end + context "with included builds" do + + context "when only one" do + before do + stub_content_request("?ref=sha", "contents_java_with_settings.json") + stub_content_request("settings.gradle?ref=sha", "contents_java_settings_1_included_build.json") + stub_content_request("build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included?ref=sha", "contents_java_with_settings.json") + stub_content_request("included/settings.gradle?ref=sha", "contents_java_simple_settings.json") + stub_content_request("included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + end + + it "fetches all buildfiles" do + expect(file_fetcher_instance.files.map(&:name)). + to match_array(%w( + build.gradle settings.gradle + app/build.gradle + included/build.gradle included/settings.gradle + included/app/build.gradle + )) + end + end + + context "when multiple" do + before do + stub_content_request("?ref=sha", "contents_java_with_settings.json") + stub_content_request("settings.gradle?ref=sha", "contents_java_settings_2_included_builds.json") + stub_content_request("build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included?ref=sha", "contents_java_with_settings.json") + stub_content_request("included/settings.gradle?ref=sha", "contents_java_simple_settings.json") + stub_content_request("included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included2?ref=sha", "contents_java_with_settings.json") + stub_content_request("included2/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included2/settings.gradle?ref=sha", "contents_java_simple_settings.json") + stub_content_request("included2/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + end + + it "fetches all buildfiles" do + expect(file_fetcher_instance.files.map(&:name)). + to match_array(%w( + build.gradle settings.gradle + app/build.gradle + included/build.gradle included/settings.gradle + included/app/build.gradle + included2/build.gradle included2/settings.gradle + included2/app/build.gradle + )) + end + end + + context "when nested included builds" do + before do + stub_content_request("?ref=sha", "contents_java_with_settings.json") + stub_content_request("settings.gradle?ref=sha", "contents_java_settings_1_included_build.json") + stub_content_request("build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included?ref=sha", "contents_java_with_settings.json") + stub_content_request("included/settings.gradle?ref=sha", "contents_java_settings_1_included_build.json") + stub_content_request("included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included/included?ref=sha", "contents_java_with_settings.json") + stub_content_request("included/included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included/included/settings.gradle?ref=sha", "contents_java_settings_1_included_build.json") + stub_content_request("included/included/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included/included/included?ref=sha", "contents_java_with_settings.json") + stub_content_request("included/included/included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included/included/included/settings.gradle?ref=sha", "contents_java_simple_settings.json") + stub_content_request("included/included/included/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + end + + it "fetches all buildfiles transitively" do + expect(file_fetcher_instance.files.map(&:name)). + to match_array(%w( + build.gradle settings.gradle + app/build.gradle + included/build.gradle included/settings.gradle + included/app/build.gradle + included/included/build.gradle included/included/settings.gradle + included/included/app/build.gradle + included/included/included/build.gradle + included/included/included/settings.gradle + included/included/included/app/build.gradle + )) + end + end + + context "containing a script plugin" do + before do + stub_content_request("?ref=sha", "contents_java_with_settings.json") + stub_content_request("settings.gradle?ref=sha", "contents_java_settings_1_included_build.json") + stub_content_request("build.gradle?ref=sha", "contents_java_buildfile_with_script_plugins.json") + stub_content_request("gradle/dependencies.gradle?ref=sha", "contents_java_simple_settings.json") + stub_content_request("app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included?ref=sha", "contents_java.json") + stub_content_request("included/build.gradle?ref=sha", "contents_java_buildfile_with_script_plugins.json") + stub_content_request("included/gradle/dependencies.gradle?ref=sha", "contents_java_simple_settings.json") + end + + it "fetches script plugin of main and included build" do + expect(file_fetcher_instance.files.map(&:name)). + to match_array(%w( + settings.gradle build.gradle + app/build.gradle + gradle/dependencies.gradle + included/build.gradle + included/gradle/dependencies.gradle + )) + end + end + end + context "only a settings.gradle" do before do stub_content_request("?ref=sha", "contents_java_only_settings.json") diff --git a/gradle/spec/fixtures/github/contents_java_settings_1_included_build.json b/gradle/spec/fixtures/github/contents_java_settings_1_included_build.json new file mode 100644 index 00000000000..59496bdc855 --- /dev/null +++ b/gradle/spec/fixtures/github/contents_java_settings_1_included_build.json @@ -0,0 +1,18 @@ +{ + "name": "settings.gradle", + "path": "settings.gradle", + "sha": "e7b4def49cb53d9aa04228dd3edb14c9e635e003", + "size": 15, + "url": "https://api.github.com/repos/git/git/contents/settings.gradle?ref=develop", + "html_url": "https://github.com/git/git/blob/develop/settings.gradle", + "git_url": "https://api.github.com/repos/git/git/git/blobs/e7b4def49cb53d9aa04228dd3edb14c9e635e003", + "download_url": "https://raw.githubusercontent.com/git/git/develop/settings.gradle", + "type": "file", + "content": "aW5jbHVkZUJ1aWxkICcuL2luY2x1ZGVkJwppbmNsdWRlICc6YXBwJw==\n", + "encoding": "base64", + "_links": { + "self": "https://api.github.com/repos/git/git/contents/settings.gradle?ref=develop", + "git": "https://api.github.com/repos/git/git/git/blobs/e7b4def49cb53d9aa04228dd3edb14c9e635e003", + "html": "https://github.com/git/git/blob/develop/settings.gradle" + } +} diff --git a/gradle/spec/fixtures/github/contents_java_settings_2_included_builds.json b/gradle/spec/fixtures/github/contents_java_settings_2_included_builds.json new file mode 100644 index 00000000000..9c7067cdd07 --- /dev/null +++ b/gradle/spec/fixtures/github/contents_java_settings_2_included_builds.json @@ -0,0 +1,18 @@ +{ + "name": "settings.gradle", + "path": "settings.gradle", + "sha": "e7b4def49cb53d9aa04228dd3edb14c9e635e003", + "size": 15, + "url": "https://api.github.com/repos/git/git/contents/settings.gradle?ref=develop", + "html_url": "https://github.com/git/git/blob/develop/settings.gradle", + "git_url": "https://api.github.com/repos/git/git/git/blobs/e7b4def49cb53d9aa04228dd3edb14c9e635e003", + "download_url": "https://raw.githubusercontent.com/git/git/develop/settings.gradle", + "type": "file", + "content": "aW5jbHVkZUJ1aWxkICcuL2luY2x1ZGVkJwppbmNsdWRlQnVpbGQgIi4vaW5jbHVkZWQyLyIKCmluY2x1ZGUgJzphcHAnCg==\n", + "encoding": "base64", + "_links": { + "self": "https://api.github.com/repos/git/git/contents/settings.gradle?ref=develop", + "git": "https://api.github.com/repos/git/git/git/blobs/e7b4def49cb53d9aa04228dd3edb14c9e635e003", + "html": "https://github.com/git/git/blob/develop/settings.gradle" + } +} From 81fff78e3251f1d650f6410e67dc57617123fca1 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Fri, 22 Apr 2022 15:26:48 +0100 Subject: [PATCH 4/6] Support buildSrc as an included build Support all already accepted files for buildSrc directories. A buildSrc directory is an implicitly included build if present. https://docs.gradle.org/current/userguide/organizing_gradle_projects.html#sec:build_sources --- gradle/lib/dependabot/gradle/file_fetcher.rb | 23 +++++++-- .../dependabot/gradle/file_fetcher_spec.rb | 45 +++++++++++++++-- ...tents_java_settings_explicit_buildsrc.json | 18 +++++++ .../github/contents_java_with_buildsrc.json | 34 +++++++++++++ ...tents_java_with_buildsrc_and_settings.json | 50 +++++++++++++++++++ 5 files changed, 160 insertions(+), 10 deletions(-) create mode 100644 gradle/spec/fixtures/github/contents_java_settings_explicit_buildsrc.json create mode 100644 gradle/spec/fixtures/github/contents_java_with_buildsrc.json create mode 100644 gradle/spec/fixtures/github/contents_java_with_buildsrc_and_settings.json diff --git a/gradle/lib/dependabot/gradle/file_fetcher.rb b/gradle/lib/dependabot/gradle/file_fetcher.rb index 6800064d642..ce8092d6fd7 100644 --- a/gradle/lib/dependabot/gradle/file_fetcher.rb +++ b/gradle/lib/dependabot/gradle/file_fetcher.rb @@ -41,12 +41,25 @@ def all_buildfiles_in_build(root_dir) end def included_builds(root_dir) - return [] unless settings_file(root_dir) + builds = [] + + # buildSrc is implicit: included but not declared in settings.gradle + buildsrc = repo_contents(dir: root_dir, raise_errors: false). + find { |item| item.type == 'dir' && item.name == 'buildSrc' } + builds << clean_join(root_dir, "buildSrc") if buildsrc + + return builds unless settings_file(root_dir) - SettingsFileParser. + builds += SettingsFileParser. new(settings_file: settings_file(root_dir)). included_build_paths. - map { |p| File.join(root_dir, p) } + map { |p| clean_join(root_dir, p) } + + builds.uniq + end + + def clean_join(*parts) + Pathname.new(File.join(*parts)).cleanpath.to_path end def subproject_buildfiles(root_dir) @@ -96,7 +109,7 @@ def dependency_script_plugins(root_dir) def check_required_files_present(files) return if files.any? - path = Pathname.new(File.join(directory, "build.gradle")).cleanpath.to_path + path = clean_join(directory, "build.gradle") path += "(.kts)?" raise Dependabot::DependencyFileNotFound, path end @@ -120,7 +133,7 @@ def settings_file(dir) def find_first(dir, supported_names) paths = supported_names. - map { |name| File.join(dir, name) }. + map { |name| clean_join(dir, name) }. each do |path| return cached_files[path] || next end diff --git a/gradle/spec/dependabot/gradle/file_fetcher_spec.rb b/gradle/spec/dependabot/gradle/file_fetcher_spec.rb index 8597777c1f1..7d7eea1711c 100644 --- a/gradle/spec/dependabot/gradle/file_fetcher_spec.rb +++ b/gradle/spec/dependabot/gradle/file_fetcher_spec.rb @@ -82,6 +82,42 @@ def stub_content_request(path, fixture) context "with included builds" do + context "when has buildSrc" do + before do + stub_content_request("buildSrc?ref=sha", "contents_java.json") + stub_content_request("buildSrc/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + end + + context "implicitly included" do + before do + stub_content_request("?ref=sha", "contents_java_with_buildsrc.json") + end + + it "fetches all buildfiles" do + expect(file_fetcher_instance.files.map(&:name)). + to match_array(%w(build.gradle buildSrc/build.gradle)) + end + end + + context "explicitly included" do + before do + stub_content_request("?ref=sha", "contents_java_with_buildsrc_and_settings.json") + stub_content_request("settings.gradle?ref=sha", "contents_java_settings_explicit_buildsrc.json") + stub_content_request("included?ref=sha", "contents_java.json") + stub_content_request("included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + end + + it "doesn't fetch buildSrc buildfiles twice" do + expect(file_fetcher_instance.files.map(&:name)). + to match_array(%w( + build.gradle settings.gradle + buildSrc/build.gradle + included/build.gradle + )) + end + end + end + context "when only one" do before do stub_content_request("?ref=sha", "contents_java_with_settings.json") @@ -148,10 +184,10 @@ def stub_content_request(path, fixture) stub_content_request("included/included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") stub_content_request("included/included/settings.gradle?ref=sha", "contents_java_settings_1_included_build.json") stub_content_request("included/included/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") - stub_content_request("included/included/included?ref=sha", "contents_java_with_settings.json") + stub_content_request("included/included/included?ref=sha", "contents_java_with_buildsrc.json") stub_content_request("included/included/included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") - stub_content_request("included/included/included/settings.gradle?ref=sha", "contents_java_simple_settings.json") - stub_content_request("included/included/included/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included/included/included/buildSrc?ref=sha", "contents_java.json") + stub_content_request("included/included/included/buildSrc/build.gradle?ref=sha", "contents_java_basic_buildfile.json") end it "fetches all buildfiles transitively" do @@ -164,8 +200,7 @@ def stub_content_request(path, fixture) included/included/build.gradle included/included/settings.gradle included/included/app/build.gradle included/included/included/build.gradle - included/included/included/settings.gradle - included/included/included/app/build.gradle + included/included/included/buildSrc/build.gradle )) end end diff --git a/gradle/spec/fixtures/github/contents_java_settings_explicit_buildsrc.json b/gradle/spec/fixtures/github/contents_java_settings_explicit_buildsrc.json new file mode 100644 index 00000000000..b75ddc56073 --- /dev/null +++ b/gradle/spec/fixtures/github/contents_java_settings_explicit_buildsrc.json @@ -0,0 +1,18 @@ +{ + "name": "settings.gradle", + "path": "settings.gradle", + "sha": "e7b4def49cb53d9aa04228dd3edb14c9e635e003", + "size": 15, + "url": "https://api.github.com/repos/git/git/contents/settings.gradle?ref=develop", + "html_url": "https://github.com/git/git/blob/develop/settings.gradle", + "git_url": "https://api.github.com/repos/git/git/git/blobs/e7b4def49cb53d9aa04228dd3edb14c9e635e003", + "download_url": "https://raw.githubusercontent.com/git/git/develop/settings.gradle", + "type": "file", + "content": "aW5jbHVkZUJ1aWxkICcuL2J1aWxkU3JjJwppbmNsdWRlQnVpbGQgJy4vaW5jbHVkZWQvJwo=\n", + "encoding": "base64", + "_links": { + "self": "https://api.github.com/repos/git/git/contents/settings.gradle?ref=develop", + "git": "https://api.github.com/repos/git/git/git/blobs/e7b4def49cb53d9aa04228dd3edb14c9e635e003", + "html": "https://github.com/git/git/blob/develop/settings.gradle" + } +} diff --git a/gradle/spec/fixtures/github/contents_java_with_buildsrc.json b/gradle/spec/fixtures/github/contents_java_with_buildsrc.json new file mode 100644 index 00000000000..d4ff289092e --- /dev/null +++ b/gradle/spec/fixtures/github/contents_java_with_buildsrc.json @@ -0,0 +1,34 @@ +[ + { + "name": "build.gradle", + "path": "build.gradle", + "sha": "f036f9f0357a50b02a422923f0bb0c3719eaf8ae", + "size": 2355, + "url": "https://api.github.com/repos/git/git/contents/build.gradle?ref=main", + "html_url": "https://github.com/git/git/blob/main/build.gradle", + "git_url": "https://api.github.com/repos/git/git/git/blobs/f036f9f0357a50b02a422923f0bb0c3719eaf8ae", + "download_url": "https://raw.githubusercontent.com/git/git/main/build.gradle", + "type": "file", + "_links": { + "self": "https://api.github.com/repos/git/git/contents/build.gradle?ref=main", + "git": "https://api.github.com/repos/git/git/git/blobs/f036f9f0357a50b02a422923f0bb0c3719eaf8ae", + "html": "https://github.com/git/git/blob/main/build.gradle" + } + }, + { + "name": "buildSrc", + "path": "buildSrc", + "sha": "5df7f650b80fef281199e7d1c6bf53c6a5280802", + "size": 0, + "url": "https://api.github.com/repos/git/git/contents/dependencies?ref=main", + "html_url": "https://github.com/git/git/tree/main/dependencies", + "git_url": "https://api.github.com/repos/git/git/git/trees/5df7f650b80fef281199e7d1c6bf53c6a5280802", + "download_url": null, + "type": "dir", + "_links": { + "self": "https://api.github.com/repos/git/git/contents/dependencies?ref=main", + "git": "https://api.github.com/repos/git/git/git/trees/5df7f650b80fef281199e7d1c6bf53c6a5280802", + "html": "https://github.com/git/git/tree/main/dependencies" + } + } +] diff --git a/gradle/spec/fixtures/github/contents_java_with_buildsrc_and_settings.json b/gradle/spec/fixtures/github/contents_java_with_buildsrc_and_settings.json new file mode 100644 index 00000000000..f9a86b43c61 --- /dev/null +++ b/gradle/spec/fixtures/github/contents_java_with_buildsrc_and_settings.json @@ -0,0 +1,50 @@ +[ + { + "name": "build.gradle", + "path": "build.gradle", + "sha": "f036f9f0357a50b02a422923f0bb0c3719eaf8ae", + "size": 2355, + "url": "https://api.github.com/repos/git/git/contents/build.gradle?ref=main", + "html_url": "https://github.com/git/git/blob/main/build.gradle", + "git_url": "https://api.github.com/repos/git/git/git/blobs/f036f9f0357a50b02a422923f0bb0c3719eaf8ae", + "download_url": "https://raw.githubusercontent.com/git/git/main/build.gradle", + "type": "file", + "_links": { + "self": "https://api.github.com/repos/git/git/contents/build.gradle?ref=main", + "git": "https://api.github.com/repos/git/git/git/blobs/f036f9f0357a50b02a422923f0bb0c3719eaf8ae", + "html": "https://github.com/git/git/blob/main/build.gradle" + } + }, + { + "name": "settings.gradle", + "path": "settings.gradle", + "sha": "f036f9f0357a50b02a422923f0bb0c3719eaf8ae", + "size": 2355, + "url": "https://api.github.com/repos/git/git/contents/build.gradle?ref=main", + "html_url": "https://github.com/git/git/blob/main/build.gradle", + "git_url": "https://api.github.com/repos/git/git/git/blobs/f036f9f0357a50b02a422923f0bb0c3719eaf8ae", + "download_url": "https://raw.githubusercontent.com/git/git/main/build.gradle", + "type": "file", + "_links": { + "self": "https://api.github.com/repos/git/git/contents/build.gradle?ref=main", + "git": "https://api.github.com/repos/git/git/git/blobs/f036f9f0357a50b02a422923f0bb0c3719eaf8ae", + "html": "https://github.com/git/git/blob/main/build.gradle" + } + }, + { + "name": "buildSrc", + "path": "buildSrc", + "sha": "5df7f650b80fef281199e7d1c6bf53c6a5280802", + "size": 0, + "url": "https://api.github.com/repos/git/git/contents/dependencies?ref=main", + "html_url": "https://github.com/git/git/tree/main/dependencies", + "git_url": "https://api.github.com/repos/git/git/git/trees/5df7f650b80fef281199e7d1c6bf53c6a5280802", + "download_url": null, + "type": "dir", + "_links": { + "self": "https://api.github.com/repos/git/git/contents/dependencies?ref=main", + "git": "https://api.github.com/repos/git/git/git/trees/5df7f650b80fef281199e7d1c6bf53c6a5280802", + "html": "https://github.com/git/git/tree/main/dependencies" + } + } +] From 0425b03b63a666820f6daf1a53d47f02dbe1f090 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Wed, 18 May 2022 22:54:29 +0100 Subject: [PATCH 5/6] Correct rubocop offenses --- gradle/lib/dependabot/gradle/file_fetcher.rb | 22 +++++++++---------- .../file_fetcher/settings_file_parser_spec.rb | 4 ++-- .../dependabot/gradle/file_fetcher_spec.rb | 7 +++--- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/gradle/lib/dependabot/gradle/file_fetcher.rb b/gradle/lib/dependabot/gradle/file_fetcher.rb index ce8092d6fd7..767e1afd124 100644 --- a/gradle/lib/dependabot/gradle/file_fetcher.rb +++ b/gradle/lib/dependabot/gradle/file_fetcher.rb @@ -36,8 +36,8 @@ def all_buildfiles_in_build(root_dir) files = [buildfile(root_dir), settings_file(root_dir)].compact files += subproject_buildfiles(root_dir) files += dependency_script_plugins(root_dir) - files += included_builds(root_dir). - flat_map { |dir| all_buildfiles_in_build(dir) } + files + included_builds(root_dir). + flat_map { |dir| all_buildfiles_in_build(dir) } end def included_builds(root_dir) @@ -45,15 +45,15 @@ def included_builds(root_dir) # buildSrc is implicit: included but not declared in settings.gradle buildsrc = repo_contents(dir: root_dir, raise_errors: false). - find { |item| item.type == 'dir' && item.name == 'buildSrc' } + find { |item| item.type == "dir" && item.name == "buildSrc" } builds << clean_join(root_dir, "buildSrc") if buildsrc return builds unless settings_file(root_dir) builds += SettingsFileParser. - new(settings_file: settings_file(root_dir)). - included_build_paths. - map { |p| clean_join(root_dir, p) } + new(settings_file: settings_file(root_dir)). + included_build_paths. + map { |p| clean_join(root_dir, p) } builds.uniq end @@ -133,15 +133,15 @@ def settings_file(dir) def find_first(dir, supported_names) paths = supported_names. - map { |name| clean_join(dir, name) }. - each do |path| - return cached_files[path] || next - end + map { |name| clean_join(dir, name) }. + each do |path| + return cached_files[path] || next + end fetch_first_if_present(paths) end def cached_files - @cached_files ||= Hash.new + @cached_files ||= {} end def fetch_first_if_present(paths) diff --git a/gradle/spec/dependabot/gradle/file_fetcher/settings_file_parser_spec.rb b/gradle/spec/dependabot/gradle/file_fetcher/settings_file_parser_spec.rb index 7a1ebf5e1d2..3c5f22e87d2 100644 --- a/gradle/spec/dependabot/gradle/file_fetcher/settings_file_parser_spec.rb +++ b/gradle/spec/dependabot/gradle/file_fetcher/settings_file_parser_spec.rb @@ -10,7 +10,7 @@ Dependabot::DependencyFile.new( name: settings_file_name, content: fixture("settings_files", fixture_name) - ) + ) end let(:settings_file_name) { "settings.gradle" } let(:fixture_name) { "simple_settings.gradle" } @@ -128,7 +128,7 @@ end end - # TODO context "with commented out included build declarations" + # TODO: context "with commented out included build declarations" context "when kotlin" do let(:settings_file_name) { "settings.gradle.kts" } diff --git a/gradle/spec/dependabot/gradle/file_fetcher_spec.rb b/gradle/spec/dependabot/gradle/file_fetcher_spec.rb index 7d7eea1711c..82df84a7a51 100644 --- a/gradle/spec/dependabot/gradle/file_fetcher_spec.rb +++ b/gradle/spec/dependabot/gradle/file_fetcher_spec.rb @@ -81,7 +81,6 @@ def stub_content_request(path, fixture) end context "with included builds" do - context "when has buildSrc" do before do stub_content_request("buildSrc?ref=sha", "contents_java.json") @@ -182,12 +181,14 @@ def stub_content_request(path, fixture) stub_content_request("included/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") stub_content_request("included/included?ref=sha", "contents_java_with_settings.json") stub_content_request("included/included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") - stub_content_request("included/included/settings.gradle?ref=sha", "contents_java_settings_1_included_build.json") + stub_content_request("included/included/settings.gradle?ref=sha", + "contents_java_settings_1_included_build.json") stub_content_request("included/included/app/build.gradle?ref=sha", "contents_java_basic_buildfile.json") stub_content_request("included/included/included?ref=sha", "contents_java_with_buildsrc.json") stub_content_request("included/included/included/build.gradle?ref=sha", "contents_java_basic_buildfile.json") stub_content_request("included/included/included/buildSrc?ref=sha", "contents_java.json") - stub_content_request("included/included/included/buildSrc/build.gradle?ref=sha", "contents_java_basic_buildfile.json") + stub_content_request("included/included/included/buildSrc/build.gradle?ref=sha", + "contents_java_basic_buildfile.json") end it "fetches all buildfiles transitively" do From 4cd56778f6565178cd680d5c4c33f980299e7066 Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Fri, 30 Sep 2022 16:44:39 -0400 Subject: [PATCH 6/6] filter_map instead of map and compact --- gradle/lib/dependabot/gradle/file_fetcher.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gradle/lib/dependabot/gradle/file_fetcher.rb b/gradle/lib/dependabot/gradle/file_fetcher.rb index 767e1afd124..62afbf98196 100644 --- a/gradle/lib/dependabot/gradle/file_fetcher.rb +++ b/gradle/lib/dependabot/gradle/file_fetcher.rb @@ -70,7 +70,7 @@ def subproject_buildfiles(root_dir) new(settings_file: settings_file(root_dir)). subproject_paths - subproject_paths.map do |path| + subproject_paths.filter_map do |path| if @buildfile_name fetch_file_from_host(File.join(root_dir, path, @buildfile_name)) else @@ -79,7 +79,7 @@ def subproject_buildfiles(root_dir) rescue Dependabot::DependencyFileNotFound # Gradle itself doesn't worry about missing subprojects, so we don't nil - end.compact + end end # rubocop:disable Metrics/PerceivedComplexity