Skip to content

Commit 3f5e0cc

Browse files
sypharsyphar
committed
Merge branch 'fix-parse-dependences' into cargo-metadata
2 parents 3834dec + 7aacaec commit 3f5e0cc

File tree

6 files changed

+168
-91
lines changed

6 files changed

+168
-91
lines changed

‎src/db/add_package.rs‎

Lines changed: 7 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
11
use crate::{
2-
db::types::{BuildStatus, Feature, version::Version},
2+
db::types::{
3+
BuildId, BuildStatus, CrateId, Feature, ReleaseId, dependencies::ReleaseDependencyList,
4+
version::Version,
5+
},
36
docbuilder::DocCoverage,
47
error::Result,
58
registry_api::{CrateData, CrateOwner, ReleaseData},
69
storage::CompressionAlgorithm,
7-
utils::{cargo_metadata::PackageExt as _, rustc_version::parse_rustc_date},
10+
utils::{MetadataPackage, rustc_version::parse_rustc_date},
811
web::crate_details::{latest_release, releases_for_crate},
912
};
1013
use anyhow::{Context, anyhow};
11-
use cargo_metadata::{Dependency, DependencyBuilder, DependencyKind};
12-
use derive_more::{Deref, Display};
1314
use futures_util::stream::TryStreamExt;
14-
use semver::VersionReq;
15-
use serde::{Deserialize, Serialize};
1615
use serde_json::Value;
1716
use slug::slugify;
1817
use std::{
@@ -23,59 +22,6 @@ use std::{
2322
};
2423
use tracing::{debug, error, info, instrument};
2524

26-
#[derive(Debug, Clone, Copy, Display, PartialEq, Eq, Hash, Serialize, sqlx::Type)]
27-
#[sqlx(transparent)]
28-
pub struct CrateId(pub i32);
29-
30-
#[derive(Debug, Clone, Copy, Display, PartialEq, Eq, Hash, Serialize, sqlx::Type)]
31-
#[sqlx(transparent)]
32-
pub struct ReleaseId(pub i32);
33-
34-
#[derive(Debug, Clone, Copy, Display, PartialEq, Eq, Hash, Serialize, sqlx::Type)]
35-
#[sqlx(transparent)]
36-
pub struct BuildId(pub i32);
37-
38-
type Dep = (String, VersionReq, Option<DependencyKind>, Option<bool>);
39-
40-
/// A crate dependency in our internal representation for releases.dependencies json.
41-
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Deref)]
42-
#[serde(from = "Dep", into = "Dep")]
43-
pub(crate) struct ReleaseDependency(Dependency);
44-
45-
impl ReleaseDependency {
46-
pub(crate) fn into_inner(self) -> Dependency {
47-
self.0
48-
}
49-
}
50-
51-
impl From<Dep> for ReleaseDependency {
52-
fn from((name, req, kind, optional): Dep) -> Self {
53-
ReleaseDependency(
54-
DependencyBuilder::default()
55-
.name(name)
56-
.source(None)
57-
.req(req)
58-
.kind(kind.unwrap_or_default())
59-
.optional(optional.unwrap_or(false))
60-
.uses_default_features(false)
61-
.features(vec![])
62-
.target(None)
63-
.rename(None)
64-
.registry(None)
65-
.path(None)
66-
.build()
67-
.expect("we know the data is correct"),
68-
)
69-
}
70-
}
71-
72-
impl From<ReleaseDependency> for Dep {
73-
fn from(rd: ReleaseDependency) -> Self {
74-
let d = rd.0;
75-
(d.name, d.req, Some(d.kind), Some(d.optional))
76-
}
77-
}
78-
7925
/// Adds a package into database.
8026
///
8127
/// Package must be built first.
@@ -102,7 +48,7 @@ pub(crate) async fn finish_release(
10248
source_size: u64,
10349
) -> Result<()> {
10450
debug!("updating release data");
105-
let dependencies = convert_dependencies(metadata_pkg);
51+
let dependencies: ReleaseDependencyList = metadata_pkg.dependencies.clone().into();
10652
let rustdoc = get_rustdoc(metadata_pkg, source_dir).unwrap_or(None);
10753
let readme = get_readme(metadata_pkg, source_dir).unwrap_or(None);
10854
let features = get_features(metadata_pkg);
@@ -137,7 +83,7 @@ pub(crate) async fn finish_release(
13783
WHERE id = $1"#,
13884
release_id.0,
13985
registry_data.release_time,
140-
serde_json::to_value(dependencies)?,
86+
serde_json::to_value(&dependencies)?,
14187
metadata_pkg.package_name(),
14288
registry_data.yanked,
14389
has_docs,
@@ -436,22 +382,6 @@ pub(crate) async fn initialize_build(
436382
Ok(build_id)
437383
}
438384

439-
fn convert_dependencies(
440-
pkg: &cargo_metadata::Package,
441-
) -> Vec<(String, VersionReq, DependencyKind, bool)> {
442-
pkg.dependencies
443-
.iter()
444-
.map(|dependency| {
445-
(
446-
dependency.name.clone(),
447-
dependency.req.clone(),
448-
dependency.kind,
449-
dependency.optional,
450-
)
451-
})
452-
.collect()
453-
}
454-
455385
/// Reads features and converts them to Vec<Feature> with default being first
456386
fn get_features(pkg: &cargo_metadata::Package) -> Vec<Feature> {
457387
let mut features = Vec::with_capacity(pkg.features.len());

‎src/db/mod.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,16 @@ use sqlx::migrate::{Migrate, Migrator};
44

55
pub use self::add_package::update_latest_version_id;
66
pub(crate) use self::add_package::{
7-
ReleaseDependency, add_doc_coverage, finish_build, finish_release, initialize_build,
8-
initialize_crate, initialize_release, update_build_with_error,
7+
add_doc_coverage, finish_build, finish_release, initialize_build, initialize_crate,
8+
initialize_release, update_build_with_error,
99
};
1010
pub use self::{
11-
add_package::{
12-
BuildId, CrateId, ReleaseId, update_build_status, update_crate_data_in_database,
13-
},
11+
add_package::{update_build_status, update_crate_data_in_database},
1412
delete::{delete_crate, delete_version},
1513
file::{add_path_into_database, add_path_into_remote_archive},
1614
overrides::Overrides,
1715
pool::{AsyncPoolClient, Pool, PoolError},
16+
types::{BuildId, CrateId, ReleaseId},
1817
};
1918

2019
mod add_package;

‎src/db/types/dependencies.rs‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
use crate::utils::Dependency;
2+
use derive_more::Deref;
3+
use semver::VersionReq;
4+
use serde::{Deserialize, Serialize};
5+
6+
const DEFAULT_KIND: &str = "normal";
7+
8+
/// The three possible representations of a dependency in our internal JSON format
9+
/// in the `releases.dependencies` column.
10+
#[derive(Debug, Clone, Serialize, Deserialize)]
11+
#[serde(untagged)]
12+
enum Dep {
13+
Two((String, VersionReq)),
14+
Three((String, VersionReq, String)),
15+
Four((String, VersionReq, String, bool)),
16+
}
17+
18+
/// A crate dependency in our internal representation for releases.dependencies json.
19+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Deref)]
20+
#[serde(from = "Dep", into = "Dep")]
21+
pub(crate) struct ReleaseDependency(Dependency);
22+
23+
impl From<Dep> for ReleaseDependency {
24+
fn from(src: Dep) -> Self {
25+
let (name, req, kind, optional) = match src {
26+
Dep::Two((name, req)) => (name, req, DEFAULT_KIND.into(), false),
27+
Dep::Three((name, req, kind)) => (name, req, kind, false),
28+
Dep::Four((name, req, kind, optional)) => (name, req, kind, optional),
29+
};
30+
31+
ReleaseDependency(Dependency {
32+
name,
33+
req,
34+
kind: Some(kind),
35+
optional,
36+
rename: None,
37+
})
38+
}
39+
}
40+
41+
impl From<ReleaseDependency> for Dep {
42+
// dependency serialization for new releases.
43+
fn from(rd: ReleaseDependency) -> Self {
44+
let d = rd.0;
45+
Dep::Four((
46+
d.name,
47+
d.req,
48+
d.kind.unwrap_or_else(|| DEFAULT_KIND.into()),
49+
d.optional,
50+
))
51+
}
52+
}
53+
54+
#[derive(Debug, Clone, Serialize, Deserialize, Default, Deref)]
55+
#[serde(transparent)]
56+
pub(crate) struct ReleaseDependencyList(Vec<ReleaseDependency>);
57+
58+
impl<I> From<I> for ReleaseDependencyList
59+
where
60+
I: IntoIterator<Item = Dependency>,
61+
{
62+
fn from(deps: I) -> Self {
63+
Self(deps.into_iter().map(ReleaseDependency).collect())
64+
}
65+
}
66+
67+
impl ReleaseDependencyList {
68+
pub(crate) fn into_iter_dependencies(self) -> impl Iterator<Item = Dependency> {
69+
self.0.into_iter().map(|rd| rd.0)
70+
}
71+
}
72+
73+
#[cfg(test)]
74+
mod tests {
75+
use super::*;
76+
use anyhow::Result;
77+
use test_case::test_case;
78+
79+
#[test_case("[]", "[]"; "empty")]
80+
#[test_case(
81+
r#"[["vec_map", "^0.0.1"]]"#,
82+
r#"[["vec_map","^0.0.1","normal",false]]"#;
83+
"2-tuple"
84+
)]
85+
#[test_case(
86+
r#"[["vec_map", "^0.0.1", "normal" ]]"#,
87+
r#"[["vec_map","^0.0.1","normal",false]]"#;
88+
"3-tuple"
89+
)]
90+
#[test_case(
91+
r#"[["rand", "^0.9", "normal", false], ["sdl3", "^0.16", "normal", false]]"#,
92+
r#"[["rand","^0.9","normal",false],["sdl3","^0.16","normal",false]]"#;
93+
"4-tuple"
94+
)]
95+
#[test_case(
96+
r#"[["byteorder", "^0.5", "normal", false],["clippy", "^0", "normal", true]]"#,
97+
r#"[["byteorder","^0.5","normal",false],["clippy","^0","normal",true]]"#;
98+
"with optional"
99+
)]
100+
fn test_parse_release_dependency_json(input: &str, output: &str) -> Result<()> {
101+
let deps: ReleaseDependencyList = serde_json::from_str(input)?;
102+
103+
assert_eq!(serde_json::to_string(&deps)?, output);
104+
Ok(())
105+
}
106+
107+
#[test_case(r#"[["vec_map", "^0.0.1"]]"#, "normal", false)]
108+
#[test_case(r#"[["vec_map", "^0.0.1", "dev" ]]"#, "dev", false)]
109+
#[test_case(r#"[["vec_map", "^0.0.1", "dev", true ]]"#, "dev", true)]
110+
fn test_parse_dependency(
111+
input: &str,
112+
expected_kind: &str,
113+
expected_optional: bool,
114+
) -> Result<()> {
115+
let deps: ReleaseDependencyList = serde_json::from_str(input)?;
116+
let [dep] = deps.0.as_slice() else {
117+
panic!("expected exactly one dependency");
118+
};
119+
120+
assert_eq!(dep.name, "vec_map");
121+
assert_eq!(dep.req, VersionReq::parse("^0.0.1")?);
122+
assert_eq!(dep.kind.as_deref(), Some(expected_kind));
123+
assert_eq!(dep.optional, expected_optional);
124+
125+
Ok(())
126+
}
127+
}

‎src/db/types/mod.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,21 @@
1+
use derive_more::Display;
12
use serde::{Deserialize, Serialize};
23

4+
pub mod dependencies;
35
pub mod version;
46

7+
#[derive(Debug, Clone, Copy, Display, PartialEq, Eq, Hash, Serialize, sqlx::Type)]
8+
#[sqlx(transparent)]
9+
pub struct CrateId(pub i32);
10+
11+
#[derive(Debug, Clone, Copy, Display, PartialEq, Eq, Hash, Serialize, sqlx::Type)]
12+
#[sqlx(transparent)]
13+
pub struct ReleaseId(pub i32);
14+
15+
#[derive(Debug, Clone, Copy, Display, PartialEq, Eq, Hash, Serialize, sqlx::Type)]
16+
#[sqlx(transparent)]
17+
pub struct BuildId(pub i32);
18+
519
#[derive(Debug, Clone, PartialEq, Eq, Serialize, sqlx::Type)]
620
#[sqlx(type_name = "feature")]
721
pub struct Feature {

‎src/web/crate_details.rs‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::{
22
AsyncStorage,
33
db::{
4-
BuildId, CrateId, ReleaseDependency, ReleaseId,
5-
types::{BuildStatus, version::Version},
4+
BuildId, CrateId, ReleaseId,
5+
types::{BuildStatus, dependencies::ReleaseDependencyList, version::Version},
66
},
77
impl_axum_webpage,
88
registry_api::OwnerKind,
@@ -235,13 +235,19 @@ impl CrateDetails {
235235

236236
let parsed_license = krate.license.as_deref().map(super::licenses::parse_license);
237237

238-
let dependencies = krate
239-
.dependencies
240-
.and_then(|value| serde_json::from_value::<Vec<ReleaseDependency>>(value).ok())
241-
.unwrap_or_default()
242-
.into_iter()
243-
.map(|rdep| rdep.into_inner())
244-
.collect();
238+
let dependencies = if let Some(value) = krate.dependencies {
239+
match serde_json::from_value::<ReleaseDependencyList>(value) {
240+
Ok(list) => list.into_iter_dependencies().collect(),
241+
Err(_) => {
242+
// NOTE: we sometimes have invalid semver-requirement strings the database
243+
// (at the time writing, 14 releases out of 2 million).
244+
// We silently ignore those here.
245+
Vec::new()
246+
}
247+
}
248+
} else {
249+
Vec::new()
250+
};
245251

246252
let mut crate_details = CrateDetails {
247253
name: krate.name,

‎src/web/rustdoc.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2678,6 +2678,7 @@ mod test {
26782678
.await
26792679
.get("/testing/0.1.0/testing/")
26802680
.await?
2681+
.error_for_status()?
26812682
.text()
26822683
.await?
26832684
));

0 commit comments

Comments
 (0)