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

Revert "Support retrieving workspace of path dependencies in cargo (#10550)" #10599

Merged
merged 2 commits into from
Sep 13, 2024
Merged

Revert "Support retrieving workspace of path dependencies in cargo (#10550)" #10599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7124eb
Revert "Support retrieving workspace of path dependencies in cargo (#…
Jefffrey Sep 13, 2024
0cb0d04
Merge branch 'main' into revert-cargo-path-dependency-find
abdulapopoola Sep 13, 2024
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
66 changes: 1 addition & 65 deletions cargo/lib/dependabot/cargo/file_fetcher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,7 @@ def fetch_path_dependency_files(file:, previously_fetched_files:)
file: fetched_file,
previously_fetched_files: previously_fetched_files
)

# If this path dependency file is a workspace member that inherits from
# its root workspace, we search for the root to include it so Cargo can
# resolve the path dependency file manifest properly.
root = find_workspace_root(fetched_file, file) if workspace_member?(parsed_file(fetched_file))

[fetched_file, *grandchild_requirement_files, root]
[fetched_file, *grandchild_requirement_files]
rescue Dependabot::DependencyFileNotFound
next unless required_path?(file, path)

Expand Down Expand Up @@ -224,64 +218,6 @@ def replacement_path_dependency_paths_from_file(file)
paths
end

# See if this Cargo manifest inherits any property from a workspace
# (e.g. edition = { workspace = true }).
def workspace_member?(hash)
hash.each do |key, value|
if key == "workspace" && value == true
return true
elsif value.is_a?(Hash)
return workspace_member?(value)
end
end
false
end

# Find workspace root of this workspace member, first via package.workspace
# manifest key if present, otherwise resort to searching parent directories
# up till the repository root.
#
# original_manifest used for providing a useful error message.
sig do
params(workspace_member: Dependabot::DependencyFile,
original_manifest: Dependabot::DependencyFile).returns(T.nilable(Dependabot::DependencyFile))
end
def find_workspace_root(workspace_member, original_manifest)
current_dir = workspace_member.name.rpartition("/").first

workspace_root_dir = parsed_file(workspace_member).dig("package", "workspace")
unless workspace_root_dir.nil?
workspace_root = fetch_file_from_host(
File.join(current_dir, workspace_root_dir, "Cargo.toml"),
fetch_submodules: true
)
return workspace_root if parsed_file(workspace_root)["workspace"]

msg = "Could not resolve workspace root for path dependency " \
"#{workspace_member.path} of #{original_manifest.path}"
raise Dependabot::DependencyFileNotEvaluatable, msg
end

parent_dirs = current_dir.scan("/").length - 1
while parent_dirs >= 0
current_dir = File.join(current_dir, "..")
begin
parent_manifest = fetch_file_from_host(
File.join(current_dir, "Cargo.toml"),
fetch_submodules: true
)
return parent_manifest if parsed_file(parent_manifest)["workspace"]
rescue Dependabot::DependencyFileNotFound
# Cargo.toml not found in this parent, keep searching up
end
parent_dirs -= 1
end

msg = "Could not resolve workspace root for path dependency " \
"#{workspace_member.path} of #{original_manifest.path}"
raise Dependabot::DependencyFileNotEvaluatable, msg
end

def workspace_dependency_paths_from_file(file)
if parsed_file(file)["workspace"] &&
!parsed_file(file)["workspace"].key?("members")
Expand Down
160 changes: 1 addition & 159 deletions cargo/spec/dependabot/cargo/file_fetcher_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@

stub_request(:get, url + "excluded/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(status: 200, body: excluded_fixture, headers: json_header)
.to_return(status: 200, body: member_fixture, headers: json_header)
end

let(:parent_fixture) do
Expand Down Expand Up @@ -742,162 +742,4 @@
.to raise_error(Dependabot::DependencyFileNotFound)
end
end

context "with a path dependency to a workspace member" do
let(:url) do
"https://api.github.com/repos/gocardless/bump/contents/"
end

before do
# Contents of these dirs aren't important
stub_request(:get, /#{Regexp.escape(url)}detached_crate_(success|fail_1|fail_2)\?ref=sha/)
.with(headers: { "Authorization" => "token token" })
.to_return(
status: 200,
body: fixture("github", "path_dependency_workspace_member", "contents_dir_detached_crate_success.json"),
headers: json_header
)

# Ignoring any .cargo requests
stub_request(:get, %r{#{Regexp.escape(url)}\w+/\.cargo\?ref=sha})
.with(headers: { "Authorization" => "token token" })
.to_return(status: 404, headers: json_header)

# All the manifest requests
stub_request(:get, url + "detached_crate_fail_1/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(
status: 200,
body: fixture("github", "path_dependency_workspace_member",
"contents_cargo_manifest_detached_crate_fail_1.json"),
headers: json_header
)
stub_request(:get, url + "detached_crate_fail_2/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(
status: 200,
body: fixture("github", "path_dependency_workspace_member",
"contents_cargo_manifest_detached_crate_fail_2.json"),
headers: json_header
)
stub_request(:get, url + "detached_crate_success/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(
status: 200,
body: fixture("github", "path_dependency_workspace_member",
"contents_cargo_manifest_detached_crate_success.json"),
headers: json_header
)
stub_request(:get, url + "detached_workspace_member/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(
status: 200,
body: fixture("github", "path_dependency_workspace_member",
"contents_cargo_manifest_detached_workspace_member.json"),
headers: json_header
)
stub_request(:get, url + "incorrect_detached_workspace_member/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(
status: 200,
body: fixture("github", "path_dependency_workspace_member",
"contents_cargo_manifest_incorrect_detached_workspace_member.json"),
headers: json_header
)
stub_request(:get, url + "incorrect_workspace/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(
status: 200,
body: fixture("github", "path_dependency_workspace_member",
"contents_cargo_manifest_incorrect_workspace.json"),
headers: json_header
)
stub_request(:get, url + "workspace/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(
status: 200,
body: fixture("github", "path_dependency_workspace_member", "contents_cargo_manifest_workspace.json"),
headers: json_header
)
stub_request(:get, url + "workspace/nested_one/nested_two/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(
status: 200,
body: fixture("github", "path_dependency_workspace_member",
"contents_cargo_manifest_workspace_nested_one_nested_two.json"),
headers: json_header
)

# nested_one dir has nothing of interest
stub_request(:get, url + "workspace/nested_one?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(status: 200, body: "[]", headers: json_header)
stub_request(:get, url + "workspace/nested_one/Cargo.toml?ref=sha")
.with(headers: { "Authorization" => "token token" })
.to_return(status: 404, headers: json_header)
end

context "with a resolvable workspace root" do
let(:source) do
Dependabot::Source.new(
provider: "github",
repo: "gocardless/bump",
directory: "detached_crate_success/"
)
end

it "fetches the dependency successfully" do
expect(file_fetcher_instance.files.map(&:name))
.to match_array(%w(
Cargo.toml
../detached_workspace_member/Cargo.toml
../workspace/Cargo.toml
../workspace/nested_one/nested_two/Cargo.toml
))
expect(file_fetcher_instance.files.map(&:path))
.to match_array(%w(
/detached_crate_success/Cargo.toml
/detached_workspace_member/Cargo.toml
/workspace/Cargo.toml
/workspace/nested_one/nested_two/Cargo.toml
))
end
end

context "with no workspace root via parent directory search" do
let(:source) do
Dependabot::Source.new(
provider: "github",
repo: "gocardless/bump",
directory: "detached_crate_fail_1/"
)
end

it "raises a DependencyFileNotEvaluatable error" do
expect { file_fetcher_instance.files }.to raise_error(Dependabot::DependencyFileNotEvaluatable) do |error|
expect(error.message)
.to eq("Could not resolve workspace root for path dependency " \
"/incorrect_workspace/Cargo.toml of /detached_crate_fail_1/Cargo.toml")
end
end
end

context "with no workspace root via package.workspace key" do
let(:source) do
Dependabot::Source.new(
provider: "github",
repo: "gocardless/bump",
directory: "detached_crate_fail_2/"
)
end

it "raises a DependencyFileNotEvaluatable error" do
expect { file_fetcher_instance.files }.to raise_error(Dependabot::DependencyFileNotEvaluatable) do |error|
expect(error.message)
.to eq("Could not resolve workspace root for path dependency " \
"/incorrect_detached_workspace_member/Cargo.toml of /detached_crate_fail_2/Cargo.toml")
end
end
end
end
end
18 changes: 0 additions & 18 deletions ...ithub/path_dependency_workspace_member/contents_cargo_manifest_detached_crate_fail_1.json
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions ...ithub/path_dependency_workspace_member/contents_cargo_manifest_detached_crate_fail_2.json
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions ...thub/path_dependency_workspace_member/contents_cargo_manifest_detached_crate_success.json
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions ...b/path_dependency_workspace_member/contents_cargo_manifest_detached_workspace_member.json
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions ...endency_workspace_member/contents_cargo_manifest_incorrect_detached_workspace_member.json
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions .../github/path_dependency_workspace_member/contents_cargo_manifest_incorrect_workspace.json
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions ...c/fixtures/github/path_dependency_workspace_member/contents_cargo_manifest_workspace.json
View file
Open in desktop

This file was deleted.

Loading
Loading