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.

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

remove activesupport dependency by creating package_manager_version #6667

Merged
merged 2 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 4 additions & 7 deletions bin/dry-run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
require "dependabot/file_updaters"
require "dependabot/pull_request_creator"
require "dependabot/config/file_fetcher"
require "dependabot/simple_instrumentor"

require "dependabot/bundler"
require "dependabot/cargo"
Expand Down Expand Up @@ -477,7 +478,7 @@ def log_conflicting_dependencies(conflicting_dependencies)
StackProf.start(raw: true) if $options[:profile]

$network_trace_count = 0
ActiveSupport::Notifications.subscribe(/excon/) do |*args|
Dependabot::SimpleInstrumentor.subscribe do |*args|
name = args.first
$network_trace_count += 1 if name == "excon.request"

Expand All @@ -488,11 +489,6 @@ def log_conflicting_dependencies(conflicting_dependencies)
end
end

$package_manager_version_log = []
Dependabot.subscribe(Dependabot::Notifications::FILE_PARSER_PACKAGE_MANAGER_VERSION_PARSED) do |*args|
$package_manager_version_log << args.last
end

$source = Dependabot::Source.new(
provider: $options[:provider],
repo: $repo_name,
Expand Down Expand Up @@ -804,7 +800,8 @@ def security_fix?(dependency)
StackProf.results("tmp/stackprof-#{Time.now.strftime('%Y-%m-%d-%H:%M')}.dump") if $options[:profile]

puts "🌍 Total requests made: '#{$network_trace_count}'"
puts "🎈 Package manager version log: #{$package_manager_version_log.join('\n')}" if $package_manager_version_log.any?
jakecoffman marked this conversation as resolved.
Show resolved Hide resolved
package_manager = fetcher.package_manager_version
puts "🎈 Package manager version log: #{package_manager}" unless package_manager.nil?

# rubocop:enable Metrics/BlockLength

Expand Down
9 changes: 9 additions & 0 deletions bundler/lib/dependabot/bundler/file_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ def self.required_files_message
"Repo must contain either a Gemfile, a gemspec, or a gems.rb."
end

def package_manager_version
{
ecosystem: "bundler",
package_managers: {
"bundler" => Helpers.detected_bundler_version(lockfile)
}
}
end

private

def fetch_files
Expand Down
12 changes: 0 additions & 12 deletions bundler/lib/dependabot/bundler/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ def parse
dependency_set += gemspec_dependencies
dependency_set += lockfile_dependencies
check_external_code(dependency_set.dependencies)
instrument_package_manager_version
dependency_set.dependencies
end

Expand All @@ -44,17 +43,6 @@ def git_source?(dependencies)
end
end

def instrument_package_manager_version
version = Helpers.detected_bundler_version(lockfile)
Dependabot.instrument(
Notifications::FILE_PARSER_PACKAGE_MANAGER_VERSION_PARSED,
ecosystem: "bundler",
package_managers: {
"bundler" => version
}
)
end

def gemfile_dependencies
dependencies = DependencySet.new

Expand Down
13 changes: 0 additions & 13 deletions bundler/spec/dependabot/bundler/file_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -828,18 +828,5 @@
end
end
end

it "instruments the package manager version" do
events = []
Dependabot.subscribe(Dependabot::Notifications::FILE_PARSER_PACKAGE_MANAGER_VERSION_PARSED) do |*args|
events << ActiveSupport::Notifications::Event.new(*args)
end

parser.parse

expect(events.last.payload).to eq(
{ ecosystem: "bundler", package_managers: { "bundler" => PackageManagerHelper.bundler_version } }
)
end
end
end
1 change: 0 additions & 1 deletion common/dependabot-common.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ Gem::Specification.new do |spec|
spec.required_ruby_version = ">= 3.1.0"
spec.required_rubygems_version = ">= 3.3.7"

spec.add_dependency "activesupport", ">= 6.0.0"
spec.add_dependency "aws-sdk-codecommit", "~> 1.28"
spec.add_dependency "aws-sdk-ecr", "~> 1.5"
spec.add_dependency "bundler", ">= 1.16", "< 3.0.0"
Expand Down
4 changes: 4 additions & 0 deletions common/lib/dependabot/file_fetchers/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ def clone_repo_contents
raise Dependabot::RepoNotFound, source
end

def package_manager_version
nil
end

private

def fetch_file_if_present(filename, fetch_submodules: false)
Expand Down
2 changes: 0 additions & 2 deletions common/lib/dependabot/file_parsers/base.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# frozen_string_literal: true

require "dependabot/notifications"

module Dependabot
module FileParsers
class Base
Expand Down
18 changes: 0 additions & 18 deletions common/lib/dependabot/notifications.rb

This file was deleted.

4 changes: 2 additions & 2 deletions common/lib/dependabot/shared_helpers.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# frozen_string_literal: true

require "active_support/notifications"
require "digest"
require "English"
require "excon"
Expand All @@ -10,6 +9,7 @@
require "shellwords"
require "tmpdir"

require "dependabot/simple_instrumentor"
require "dependabot/utils"
require "dependabot/errors"
require "dependabot"
Expand Down Expand Up @@ -152,7 +152,7 @@ def self.excon_defaults(options = nil)
options ||= {}
headers = options.delete(:headers)
{
instrumentor: ActiveSupport::Notifications,
jakecoffman marked this conversation as resolved.
Show resolved Hide resolved
instrumentor: Dependabot::SimpleInstrumentor,
connect_timeout: 5,
write_timeout: 5,
read_timeout: 20,
Expand Down
19 changes: 19 additions & 0 deletions common/lib/dependabot/simple_instrumentor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

module Dependabot
module SimpleInstrumentor
class << self
attr_accessor :events, :subscribers

def subscribe(&block)
@subscribers ||= []
@subscribers << block
end

def instrument(name, params = {}, &block)
@subscribers&.each { |s| s.call(name, params) }
yield if block
end
end
end
end
3 changes: 2 additions & 1 deletion common/spec/dependabot/shared_helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require "spec_helper"
require "dependabot/shared_helpers"
require "dependabot/simple_instrumentor"

RSpec.describe Dependabot::SharedHelpers do
let(:spec_root) { File.join(File.dirname(__FILE__), "..") }
Expand Down Expand Up @@ -301,7 +302,7 @@ def existing_tmp_folders

it "includes the defaults" do
expect(subject).to eq(
instrumentor: ActiveSupport::Notifications,
instrumentor: Dependabot::SimpleInstrumentor,
connect_timeout: 5,
write_timeout: 5,
read_timeout: 20,
Expand Down
2 changes: 1 addition & 1 deletion maven/lib/dependabot/maven/metadata_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def repo_has_subdir_for_dep?(tmp_source)
any? { |f| dependency_artifact_id.end_with?(f.name) }
rescue Dependabot::BranchNotFound
# If we are attempting to find a branch, we should fail over to the default branch and retry once only
if tmp_source.branch.present?
unless tmp_source.branch.to_s.empty?
tmp_source.branch = nil
retry
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def versions
repositories.map do |repository_details|
url = repository_details.fetch("url")
xml = dependency_metadata(repository_details)
next [] if xml.blank?
next [] if xml.nil?

break xml.css("versions > version").
select { |node| version_class.correct?(node.content) }.
Expand Down
28 changes: 13 additions & 15 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/file_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,19 @@ def clone_repo_contents
end
end

def package_manager_version
package_managers = {}

package_managers["npm"] = Helpers.npm_version_numeric(package_lock.content) if package_lock
package_managers["yarn"] = yarn_version if yarn_version
package_managers["shrinkwrap"] = 1 if shrinkwrap

{
ecosystem: "npm",
package_managers: package_managers
}
end

private

def fetch_files
Expand All @@ -65,7 +78,6 @@ def fetch_files
fetched_files += workspace_package_jsons
fetched_files += lerna_packages
fetched_files += path_dependencies(fetched_files)
instrument_package_manager_version

fetched_files << inferred_npmrc if inferred_npmrc

Expand Down Expand Up @@ -103,20 +115,6 @@ def inferred_npmrc
@inferred_npmrc = nil
end

def instrument_package_manager_version
package_managers = {}

package_managers["npm"] = Helpers.npm_version_numeric(package_lock.content) if package_lock
package_managers["yarn"] = yarn_version if yarn_version
package_managers["shrinkwrap"] = 1 if shrinkwrap

Dependabot.instrument(
Notifications::FILE_PARSER_PACKAGE_MANAGER_VERSION_PARSED,
ecosystem: "npm",
package_managers: package_managers
)
end

def yarn_version
return @yarn_version if defined?(@yarn_version)

Expand Down
25 changes: 8 additions & 17 deletions npm_and_yarn/spec/dependabot/npm_and_yarn/file_fetcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
}]
end
let(:json_header) { { "content-type" => "application/json" } }
let(:events) { [] }

before do
allow(file_fetcher_instance).to receive(:commit).and_return("sha")
Expand Down Expand Up @@ -56,10 +55,6 @@
body: fixture("github", "package_lock_content.json"),
headers: json_header
)

Dependabot.subscribe(Dependabot::Notifications::FILE_PARSER_PACKAGE_MANAGER_VERSION_PARSED) do |*args|
events << ActiveSupport::Notifications::Event.new(*args)
end
end

context "with .yarn data stored in git-lfs" do
Expand Down Expand Up @@ -286,9 +281,8 @@
to match_array(%w(package.json yarn.lock))
end

it "instruments the yarn lockfile" do
file_fetcher_instance.files
expect(events.last.payload).to eq(
it "parses the yarn lockfile" do
expect(file_fetcher_instance.package_manager_version).to eq(
{ ecosystem: "npm", package_managers: { "yarn" => 1 } }
)
end
Expand Down Expand Up @@ -345,9 +339,8 @@
to match_array(%w(package.json npm-shrinkwrap.json))
end

it "instruments the shrinkwrap file" do
file_fetcher_instance.files
expect(events.last.payload).to eq(
it "parses the shrinkwrap file" do
expect(file_fetcher_instance.package_manager_version).to eq(
{ ecosystem: "npm", package_managers: { "shrinkwrap" => 1 } }
)
end
Expand Down Expand Up @@ -379,9 +372,8 @@
to match_array(%w(package.json package-lock.json))
end

it "instruments the npm lockfile" do
file_fetcher_instance.files
expect(events.last.payload).to eq(
it "parses the npm lockfile" do
expect(file_fetcher_instance.package_manager_version).to eq(
{ ecosystem: "npm", package_managers: { "npm" => 6 } }
)
end
Expand Down Expand Up @@ -417,9 +409,8 @@
to match_array(%w(package.json package-lock.json yarn.lock))
end

it "instruments the npm and yarn lockfiles" do
file_fetcher_instance.files
expect(events.last.payload).to eq(
it "parses the package manager version" do
expect(file_fetcher_instance.package_manager_version).to eq(
{ ecosystem: "npm", package_managers: { "npm" => 6, "yarn" => 1 } }
)
end
Expand Down
8 changes: 5 additions & 3 deletions terraform/lib/dependabot/terraform/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,12 @@ def source_from(details_hash)

def provider_source_from(source_address, name)
matches = source_address&.match(PROVIDER_SOURCE_ADDRESS)
matches = {} if matches.nil?

[
matches.try(:[], :hostname) || DEFAULT_REGISTRY,
matches.try(:[], :namespace) || DEFAULT_NAMESPACE,
matches.try(:[], :name) || name
matches[:hostname] || DEFAULT_REGISTRY,
matches[:namespace] || DEFAULT_NAMESPACE,
matches[:name] || name
]
end

Expand Down
1 change: 0 additions & 1 deletion updater/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ gem "dependabot-pub", path: "../pub"
gem "dependabot-python", path: "../python"
gem "dependabot-terraform", path: "../terraform"

gem "activesupport", "~> 6.1.7"
gem "http", "~> 5.1"
gem "octokit", "6.0.1"
gem "sentry-raven", "~> 3.1"
Expand Down
8 changes: 0 additions & 8 deletions updater/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ PATH
remote: ../common
specs:
dependabot-common (0.215.0)
activesupport (>= 6.0.0)
aws-sdk-codecommit (~> 1.28)
aws-sdk-ecr (~> 1.5)
bundler (>= 1.16, < 3.0.0)
Expand Down Expand Up @@ -118,12 +117,6 @@ PATH
GEM
remote: https://rubygems.org/
specs:
activesupport (6.1.7)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 1.6, < 2)
minitest (>= 5.1)
tzinfo (~> 2.0)
zeitwerk (~> 2.3)
addressable (2.8.1)
public_suffix (>= 2.0.2, < 6.0)
ast (2.4.2)
Expand Down Expand Up @@ -302,7 +295,6 @@ PLATFORMS
ruby

DEPENDENCIES
activesupport (~> 6.1.7)
debug (~> 1.7.1)
dependabot-bundler!
dependabot-cargo!
Expand Down
Loading