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

Refactor pnpm updater to handle devDependencies and optimize update logic #11304

Closed
wants to merge 6 commits into from
Closed
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
3 changes: 2 additions & 1 deletion common/lib/dependabot/config/update_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ def extract_base_version_from_requirement(dependency)
name: dependency.name,
version: version,
requirements: dependency.requirements,
package_manager: dependency.package_manager
package_manager: dependency.package_manager,
workspace: dependency.workspace
)
end

Expand Down
9 changes: 8 additions & 1 deletion common/lib/dependabot/dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ def self.register_name_normaliser(package_manager, name_builder)
sig { returns(T::Hash[Symbol, T.untyped]) }
attr_reader :metadata

sig { returns(T.nilable(String)) }
attr_reader :workspace

# rubocop:disable Metrics/AbcSize
# rubocop:disable Metrics/PerceivedComplexity
sig do
Expand All @@ -100,14 +103,15 @@ def self.register_name_normaliser(package_manager, name_builder)
previous_version: T.nilable(String),
previous_requirements: T.nilable(T::Array[T::Hash[T.any(Symbol, String), T.untyped]]),
directory: T.nilable(String),
workspace: T.nilable(String),
subdependency_metadata: T.nilable(T::Array[T::Hash[T.any(Symbol, String), String]]),
removed: T::Boolean,
metadata: T.nilable(T::Hash[T.any(Symbol, String), String])
).void
end
def initialize(name:, requirements:, package_manager:, version: nil,
previous_version: nil, previous_requirements: nil, directory: nil,
subdependency_metadata: [], removed: false, metadata: {})
workspace: nil, subdependency_metadata: [], removed: false, metadata: {})
@name = name
@version = T.let(
case version
Expand All @@ -126,6 +130,8 @@ def initialize(name:, requirements:, package_manager:, version: nil,
)
@package_manager = package_manager
@directory = directory
@workspace = workspace

unless top_level? || subdependency_metadata == []
@subdependency_metadata = T.let(
subdependency_metadata&.map { |h| symbolize_keys(h) },
Expand Down Expand Up @@ -166,6 +172,7 @@ def to_h
"previous_version" => previous_version,
"previous_requirements" => previous_requirements,
"directory" => directory,
"workspace" => workspace,
"package_manager" => package_manager,
"subdependency_metadata" => subdependency_metadata,
"removed" => removed? ? true : nil
Expand Down
57 changes: 57 additions & 0 deletions common/lib/dependabot/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ def self.fetcher_error_details(error)
"supported-versions": error.supported_versions
}
}
when Dependabot::ToolFeatureNotSupported
{
"error-type": "tool_feature_not_supported",
"error-detail": {
"tool-name": error.tool_name,
"tool-type": error.tool_type,
feature: error.feature
}
}
when Dependabot::BranchNotFound
{
"error-type": "branch_not_found",
Expand Down Expand Up @@ -103,6 +112,15 @@ def self.fetcher_error_details(error)
sig { params(error: StandardError).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
def self.parser_error_details(error)
case error
when Dependabot::ToolFeatureNotSupported
{
"error-type": "tool_feature_not_supported",
"error-detail": {
"tool-name": error.tool_name,
"tool-type": error.tool_type,
feature: error.feature
}
}
when Dependabot::DependencyFileNotEvaluatable
{
"error-type": "dependency_file_not_evaluatable",
Expand Down Expand Up @@ -170,6 +188,15 @@ def self.parser_error_details(error)
sig { params(error: StandardError).returns(T.nilable(T::Hash[Symbol, T.untyped])) }
def self.updater_error_details(error)
case error
when Dependabot::ToolFeatureNotSupported
{
"error-type": "tool_feature_not_supported",
"error-detail": {
"tool-name": error.tool_name,
"tool-type": error.tool_type,
feature: error.feature
}
}
when Dependabot::DependencyFileNotResolvable
{
"error-type": "dependency_file_not_resolvable",
Expand Down Expand Up @@ -300,6 +327,7 @@ def self.updater_error_details(error)
}
end
end

# rubocop:enable Metrics/MethodLength
# rubocop:enable Metrics/CyclomaticComplexity
# rubocop:enable Lint/RedundantCopDisableDirective
Expand Down Expand Up @@ -490,6 +518,35 @@ def initialize(tool_name, detected_version, supported_versions)
end
end

class ToolFeatureNotSupported < DependabotError
extend T::Sig

sig { returns(String) }
attr_reader :tool_name, :tool_type, :feature

sig do
params(
tool_name: String,
tool_type: String,
feature: String
).void
end
def initialize(tool_name:, tool_type:, feature:)
@tool_name = tool_name
@tool_type = tool_type
@feature = feature
super(build_message)
end

private

sig { returns(String) }
def build_message
"Dependabot doesn't support the feature '#{feature}' for #{tool_name} (#{tool_type}). " \
"Please refer to the documentation for supported features."
end
end

class DependencyFileNotFound < DependabotError
extend T::Sig

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ def combined_dependency(old_dep, new_dep)
version: version,
requirements: requirements,
package_manager: old_dep.package_manager,
workspace: old_dep.workspace,
metadata: old_dep.metadata,
subdependency_metadata: subdependency_metadata
)
Expand Down
1 change: 1 addition & 0 deletions common/lib/dependabot/git_commit_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ def listing_source_url
candidate_dep = Dependency.new(
name: dependency.name,
version: dependency.version,
workspace: dependency.workspace,
requirements: [],
package_manager: dependency.package_manager
)
Expand Down
2 changes: 2 additions & 0 deletions common/lib/dependabot/update_checkers/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ def updated_dependency_without_unlock
previous_version: previous_version,
previous_requirements: dependency.requirements,
package_manager: dependency.package_manager,
workspace: dependency.workspace,
metadata: dependency.metadata,
subdependency_metadata: dependency.subdependency_metadata
)
Expand All @@ -250,6 +251,7 @@ def updated_dependency_with_own_req_unlock
previous_version: previous_version,
previous_requirements: dependency.requirements,
package_manager: dependency.package_manager,
workspace: dependency.workspace,
metadata: dependency.metadata,
subdependency_metadata: dependency.subdependency_metadata
)
Expand Down
19 changes: 18 additions & 1 deletion npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ def lockfile_dependencies
lockfile_parser.parse_set
end

# rubocop:disable Metrics/MethodLength
# rubocop:disable Metrics/PerceivedComplexity
sig do
params(file: DependencyFile, type: T.untyped, name: String, requirement: String)
.returns(T.nilable(Dependency))
Expand All @@ -258,7 +260,19 @@ def build_dependency(file:, type:, name:, requirement:)

return if lockfile_details && !version
return if ignore_requirement?(requirement)
return if workspace_package_names.include?(name)

workspace_name = nil

if Dependabot::Experiments.enabled?(:enable_fix_for_pnpm_no_change_error)
workspaces = workspace_package_names

return if workspaces.include?(name)

# Extract workspace name by finding the first match in workspace_package_names
workspace_name = workspaces.find do |workspace|
file.name.start_with?(workspace + "/")
end
end

# TODO: Handle aliased packages:
# https://github.com/dependabot/dependabot-core/pull/1115
Expand All @@ -271,6 +285,7 @@ def build_dependency(file:, type:, name:, requirement:)
name: name,
version: converted_version,
package_manager: ECOSYSTEM,
workspace: workspace_name,
requirements: [{
requirement: requirement_for(requirement),
file: file.name,
Expand All @@ -279,6 +294,8 @@ def build_dependency(file:, type:, name:, requirement:)
}]
)
end
# rubocop:enable Metrics/MethodLength
# rubocop:enable Metrics/PerceivedComplexity

sig { override.void }
def check_required_files
Expand Down
32 changes: 32 additions & 0 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ def self.updated_files_regex
]
end

# rubocop:disable Metrics/AbcSize
# rubocop:disable Metrics/PerceivedComplexity
# rubocop:disable Metrics/CyclomaticComplexity
# rubocop:disable Metrics/MethodLength
sig { override.returns(T::Array[DependencyFile]) }
def updated_dependency_files
updated_files = T.let([], T::Array[DependencyFile])
Expand All @@ -56,6 +60,30 @@ def updated_dependency_files
updated_files += updated_lockfiles

if updated_files.none?

if Dependabot::Experiments.enabled?(:enable_fix_for_pnpm_no_change_error)
all_workspaces_present = dependencies.all? do |dependency|
!dependency.workspace.nil? && !dependency.workspace&.empty?
end

all_dev_dependencies = dependencies.all? do |dependency|
dependency.requirements.all? do |requirement|
requirement[:groups].include?("devDependencies")
end
end

if dependencies.any?(&:top_level?) &&
pnpm_locks.any? &&
all_workspaces_present &&
all_dev_dependencies
raise ToolFeatureNotSupported.new(
tool_name: PNPMPackageManager::NAME,
tool_type: "package_manager",
feature: "updating dev dependencies in workspaces"
)
end
end

raise NoChangeError.new(
message: "No files were updated!",
error_context: error_context(updated_files: updated_files)
Expand All @@ -72,6 +100,10 @@ def updated_dependency_files

vendor_updated_files(updated_files)
end
# rubocop:enable Metrics/AbcSize
# rubocop:enable Metrics/PerceivedComplexity
# rubocop:enable Metrics/CyclomaticComplexity
# rubocop:enable Metrics/MethodLength

private

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@
FileUtils.mkdir_p(tmp_path)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:enable_shared_helpers_command_timeout).and_return(true)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:enable_fix_for_pnpm_no_change_error).and_return(true)
end

after do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@
.with(:enable_shared_helpers_command_timeout).and_return(true)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:npm_v6_deprecation_warning).and_return(true)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:enable_fix_for_pnpm_no_change_error).and_return(true)
end

after do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@
.with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:enable_shared_helpers_command_timeout).and_return(true)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:enable_fix_for_pnpm_no_change_error).and_return(true)
end

after do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@
.with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:enable_shared_helpers_command_timeout).and_return(true)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:enable_fix_for_pnpm_no_change_error).and_return(true)
end

after do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@
.with(:enable_shared_helpers_command_timeout).and_return(true)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:npm_v6_deprecation_warning).and_return(true)
allow(Dependabot::Experiments).to receive(:enabled?)
.with(:enable_fix_for_pnpm_no_change_error).and_return(true)
end

after do
Expand Down
Loading