-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Ensure Corepack Usage for npm, pnpm, and yarn Command Execution #10944
Changes from all commits
6da1382
a566c64
b0d98c3
adc4f4e
7fa48da
7c145ec
e502b93
792d47b
d0ecf1a
fd7da7b
ee0faad
ca3be71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -172,18 +172,15 @@ def detect_package_manager | |
|
||
sig { returns(T.nilable(String)) } | ||
def name_from_lockfiles | ||
PACKAGE_MANAGER_CLASSES.each_key do |manager_name| # iterates keys in order as defined in the hash | ||
return manager_name.to_s if @lockfiles[manager_name.to_sym] | ||
end | ||
nil | ||
PACKAGE_MANAGER_CLASSES.keys.map(&:to_s).find { |manager_name| @lockfiles[manager_name.to_sym] } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix Sorbet Issue:
|
||
end | ||
|
||
sig { returns(T.nilable(String)) } | ||
def name_from_package_manager_attr | ||
return unless @manifest_package_manager | ||
|
||
PACKAGE_MANAGER_CLASSES.each_key do |manager_name| # iterates keys in order as defined in the hash | ||
return manager_name.to_s if @manifest_package_manager.start_with?("#{manager_name}@") | ||
PACKAGE_MANAGER_CLASSES.keys.map(&:to_s).find do |manager_name| | ||
@manifest_package_manager.start_with?("#{manager_name}@") | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix Sorbet Issue:
|
||
end | ||
|
||
|
@@ -255,22 +252,30 @@ def setup(name) | |
) | ||
end | ||
|
||
version ||= requested_version(name) | ||
|
||
if version | ||
raise_if_unsupported!(name, version) | ||
if Dependabot::Experiments.enabled?(:enable_corepack_for_npm_and_yarn) | ||
version ||= requested_version(name) || guessed_version(name) | ||
|
||
install(name, version) | ||
if version | ||
raise_if_unsupported!(name, version.to_s) | ||
install(name, version) | ||
end | ||
else | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This else is going to be removed in clean-up process of |
||
version = guessed_version(name) | ||
version ||= requested_version(name) | ||
|
||
if version | ||
raise_if_unsupported!(name, version.to_s) | ||
raise_if_unsupported!(name, version) | ||
|
||
install(name, version) | ||
else | ||
version = guessed_version(name) | ||
|
||
if version | ||
raise_if_unsupported!(name, version.to_s) | ||
|
||
install(name, version) if name == PNPMPackageManager::NAME | ||
install(name, version) if name == PNPMPackageManager::NAME | ||
end | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want to install npm, and yarn versions as well. We removed the condition that only install for pnpm when version is guessed. |
||
|
||
version | ||
end | ||
# rubocop:enable Metrics/CyclomaticComplexity | ||
|
@@ -299,6 +304,10 @@ def raise_if_unsupported!(name, version) | |
end | ||
|
||
def install(name, version) | ||
if Dependabot::Experiments.enabled?(:enable_corepack_for_npm_and_yarn) | ||
return Helpers.install(name, version.to_s) | ||
end | ||
|
||
Dependabot.logger.info("Installing \"#{name}@#{version}\"") | ||
|
||
SharedHelpers.run_shell_command( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,6 +63,9 @@ | |
# Variable to control the npm fallback version feature flag | ||
let(:npm_fallback_version_above_v6_enabled) { true } | ||
|
||
# Variable to control the enabling feature flag for the corepack fix | ||
let(:enable_corepack_for_npm_and_yarn) { true } | ||
|
||
before do | ||
stub_request(:get, react_dom_registry_listing_url) | ||
.to_return(status: 200, body: react_dom_registry_response) | ||
|
@@ -79,7 +82,7 @@ | |
allow(Dependabot::Experiments).to receive(:enabled?) | ||
.with(:npm_fallback_version_above_v6).and_return(npm_fallback_version_above_v6_enabled) | ||
allow(Dependabot::Experiments).to receive(:enabled?) | ||
.with(:npm_fallback_version_above_v6).and_return(npm_fallback_version_above_v6_enabled) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate! |
||
.with(:enable_corepack_for_npm_and_yarn).and_return(enable_corepack_for_npm_and_yarn) | ||
end | ||
|
||
after do | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved from
PackageManagerHelper
intoHelpers
.