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.

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

npm: detect npm 7 lockfiles #2979

Merged
merged 2 commits into from
Jan 14, 2021
Merged

npm: detect npm 7 lockfiles #2979

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
257c79d
npm: detect npm 7 lockfiles
feelepxyz Jan 13, 2021
186bb06
Add Dependabot logger
feelepxyz Jan 13, 2021
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
33 changes: 21 additions & 12 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater/npm_lockfile_updater.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "dependabot/npm_and_yarn/file_parser"
require "dependabot/npm_and_yarn/update_checker/registry_finder"
require "dependabot/npm_and_yarn/native_helpers"
require "dependabot/npm_and_yarn/helpers"
require "dependabot/shared_helpers"
require "dependabot/errors"

Expand Down Expand Up @@ -32,7 +33,7 @@ def updated_lockfile_content(lockfile)
lockfile_name = Pathname.new(lockfile.name).basename.to_s
write_temporary_dependency_files(lockfile.name)
updated_files = Dir.chdir(path) do
run_current_npm_update(lockfile_name: lockfile_name)
run_current_npm_update(lockfile_name: lockfile_name, lockfile_content: lockfile.content)
end
updated_content = updated_files.fetch(lockfile_name)
post_process_npm_lockfile(lockfile.content, updated_content)
Expand Down Expand Up @@ -107,18 +108,19 @@ def top_level_dependency_update_not_required?(dependency,
dependency.top_level? && requirements_for_path.empty?
end

def run_current_npm_update(lockfile_name:)
def run_current_npm_update(lockfile_name:, lockfile_content:)
top_level_dependency_updates = top_level_dependencies.map do |d|
{ name: d.name, version: d.version, requirements: d.requirements }
end

run_npm_updater(
lockfile_name: lockfile_name,
top_level_dependency_updates: top_level_dependency_updates
top_level_dependency_updates: top_level_dependency_updates,
lockfile_content: lockfile_content
)
end

def run_previous_npm_update(lockfile_name:)
def run_previous_npm_update(lockfile_name:, lockfile_content:)
previous_top_level_dependencies = top_level_dependencies.map do |d|
{
name: d.name,
Expand All @@ -129,25 +131,29 @@ def run_previous_npm_update(lockfile_name:)

run_npm_updater(
lockfile_name: lockfile_name,
top_level_dependency_updates: previous_top_level_dependencies
top_level_dependency_updates: previous_top_level_dependencies,
lockfile_content: lockfile_content
)
end

def run_npm_updater(lockfile_name:, top_level_dependency_updates:)
def run_npm_updater(lockfile_name:, top_level_dependency_updates:, lockfile_content:)
SharedHelpers.with_git_configured(credentials: credentials) do
if top_level_dependency_updates.any?
run_npm_top_level_updater(
lockfile_name: lockfile_name,
top_level_dependency_updates: top_level_dependency_updates
top_level_dependency_updates: top_level_dependency_updates,
lockfile_content: lockfile_content
)
else
run_npm_subdependency_updater(lockfile_name: lockfile_name)
run_npm_subdependency_updater(lockfile_name: lockfile_name, lockfile_content: lockfile_content)
end
end
end

def run_npm_top_level_updater(lockfile_name:,
top_level_dependency_updates:)
def run_npm_top_level_updater(lockfile_name:, top_level_dependency_updates:, lockfile_content:)
npm_version = Dependabot::NpmAndYarn::Helpers.npm_version(lockfile_content)
puts npm_version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jurre not entirely happy with this duplication but seems worth doing it in ruby so we can emit stats on the version when we add instrumentation. Was also thinking we could do the detection in JS land but would make instrumentation a lot more painful.

Copy link
Member

Choose a reason for hiding this comment

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

Aye, yeah it might be slick to do it in the native helpers, but I can't think of a good way to report on it if we do that 🤔


SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
function: "npm6:update",
Expand All @@ -159,7 +165,10 @@ def run_npm_top_level_updater(lockfile_name:,
)
end

def run_npm_subdependency_updater(lockfile_name:)
def run_npm_subdependency_updater(lockfile_name:, lockfile_content:)
npm_version = Dependabot::NpmAndYarn::Helpers.npm_version(lockfile_content)
puts npm_version

SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
function: "npm6:updateSubdependency",
Expand Down Expand Up @@ -341,7 +350,7 @@ def resolvable_before_update?(lockfile)
lockfile_name = Pathname.new(lockfile.name).basename.to_s
path = Pathname.new(lockfile.name).dirname.to_s
Dir.chdir(path) do
run_previous_npm_update(lockfile_name: lockfile_name)
run_previous_npm_update(lockfile_name: lockfile_name, lockfile_content: lockfile.content)
end
end

Expand Down
16 changes: 16 additions & 0 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

module Dependabot
module NpmAndYarn
module Helpers
def self.npm_version(lockfile_content)
return "npm6" unless lockfile_content
return "npm7" if JSON.parse(lockfile_content)["lockfileVersion"] == 2

"npm6"
rescue JSON::ParserError
"npm6"
end
end
end
end
6 changes: 6 additions & 0 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/conflicting_dependency_resolver.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "dependabot/errors"
require "dependabot/npm_and_yarn/file_parser"
require "dependabot/npm_and_yarn/native_helpers"
require "dependabot/npm_and_yarn/helpers"
require "dependabot/npm_and_yarn/update_checker"
require "dependabot/npm_and_yarn/update_checker/dependency_files_builder"
require "dependabot/shared_helpers"
Expand Down Expand Up @@ -43,6 +44,11 @@ def conflicting_dependencies(dependency:, target_version:)
# parser doesn't deal with at the moment.
if dependency_files_builder.package_locks.any? ||
dependency_files_builder.shrinkwraps.any?
dependency_files_builder.package_locks
package_lock = dependency_files_builder.package_locks.find { |f| f.name == "package-lock.json" }
npm_version = Dependabot::NpmAndYarn::Helpers.npm_version(package_lock&.content)
puts npm_version

SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
function: "npm6:findConflictingDependencies",
Expand Down
8 changes: 6 additions & 2 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/subdependency_version_resolver.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require "dependabot/npm_and_yarn/file_updater/npmrc_builder"
require "dependabot/npm_and_yarn/file_updater/package_json_preparer"
require "dependabot/npm_and_yarn/native_helpers"
require "dependabot/npm_and_yarn/helpers"
require "dependabot/npm_and_yarn/sub_dependency_files_filterer"
require "dependabot/npm_and_yarn/update_checker"
require "dependabot/npm_and_yarn/update_checker/dependency_files_builder"
Expand Down Expand Up @@ -60,7 +61,7 @@ def update_subdependency_in_lockfile(lockfile)
updated_files = if lockfile.name.end_with?("yarn.lock")
run_yarn_updater(path, lockfile_name)
else
run_npm_updater(path, lockfile_name)
run_npm_updater(path, lockfile_name, lockfile.content)
end

updated_files.fetch(lockfile_name)
Expand Down Expand Up @@ -107,9 +108,12 @@ def run_yarn_updater(path, lockfile_name)
sleep(rand(3.0..10.0)) && retry
end

def run_npm_updater(path, lockfile_name)
def run_npm_updater(path, lockfile_name, lockfile_content)
SharedHelpers.with_git_configured(credentials: credentials) do
Dir.chdir(path) do
npm_version = Dependabot::NpmAndYarn::Helpers.npm_version(lockfile_content)
puts npm_version
feelepxyz marked this conversation as resolved.
Show resolved Hide resolved

SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
function: "npm6:updateSubdependency",
Expand Down
5 changes: 5 additions & 0 deletions npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/version_resolver.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require "dependabot/npm_and_yarn/version"
require "dependabot/npm_and_yarn/requirement"
require "dependabot/npm_and_yarn/native_helpers"
require "dependabot/npm_and_yarn/helpers"
require "dependabot/npm_and_yarn/dependency_files_filterer"
require "dependabot/shared_helpers"
require "dependabot/errors"
Expand Down Expand Up @@ -413,6 +414,10 @@ def run_yarn_checker(path:, version:)
def run_npm_checker(path:, version:)
SharedHelpers.with_git_configured(credentials: credentials) do
Dir.chdir(path) do
package_lock = dependency_files_builder.package_locks.find { |f| f.name == "package-lock.json" }
npm_version = Dependabot::NpmAndYarn::Helpers.npm_version(package_lock&.content)
puts npm_version

SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
function: "npm6:checkPeerDependencies",
Expand Down