Skip to content

Commit c4d041c

Browse files
committed
Don't try to delete rustdoc files for binary releases
This avoids confusing errors like the following: ``` Error: failed to delete the crate Caused by: <?xml version="1.0" encoding="UTF-8"?> <Error><Code>MalformedXML</Code><Message>The XML you provided was not well-formed or did not validate against our published schema</Message><RequestId>9F280BB6EBFEC7F3</RequestId><HostId>sgrbwS6rXn83txaNR4V3+YyVm/+VhIrD1Nhaiv9og8H5hNnGq2yntXaGt45DQ8rcmopLRGoPqkg=</HostId></Error> ``` Tested locally like so: ``` $ cargo run build crate regex 1.4.1 $ cargo run build crate bat 0.12.1 $ psql postgresql://cratesfyi:password@localhost:15432 cratesfyi=# select path from files where path like '%bat%'; path ------------------------------ sources/bat/0.12.1.zip sources/bat/0.12.1.zip.index (2 rows) cratesfyi=# select path from files where path like '%regex%'; path ------------------------------- rustdoc/regex/1.4.1.zip rustdoc/regex/1.4.1.zip.index sources/regex/1.4.1.zip sources/regex/1.4.1.zip.index (4 rows) cratesfyi=# \q $ cargo run database delete version regex 1.4.1 Finished dev [unoptimized + debuginfo] target(s) in 0.15s Running `target/debug/cratesfyi database delete version regex 1.4.1` $ psql postgresql://cratesfyi:password@localhost:15432 cratesfyi=# select path from files where path like '%regex%'; path ------ (0 rows) cratesfyi=# select path from files where path like '%bat%'; path ------------------------------ sources/bat/0.12.1.zip sources/bat/0.12.1.zip.index (2 rows) cratesfyi=# \q $ cargo run database delete version bat 0.12.1 Finished dev [unoptimized + debuginfo] target(s) in 0.14s Running `target/debug/cratesfyi database delete version bat 0.12.1` $ psql postgresql://cratesfyi:password@localhost:15432 cratesfyi=# select path from files where path like '%bat%'; path ------ (0 rows) cratesfyi=# \q ```
1 parent 0bea432 commit c4d041c

File tree

2 files changed

+52
-20
lines changed

2 files changed

+52
-20
lines changed

src/bin/cratesfyi.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,9 @@ impl DatabaseSubcommand {
447447

448448
Self::Delete {
449449
command: DeleteSubcommand::Version { name, version },
450-
} => db::delete_version(&ctx, &name, &version).context("failed to delete the crate")?,
450+
} => {
451+
db::delete_version(&ctx, &name, &version).context("failed to delete the version")?
452+
}
451453
Self::Delete {
452454
command: DeleteSubcommand::Crate { name },
453455
} => db::delete_crate(&ctx, &name).context("failed to delete the crate")?,

src/db/delete.rs

+49-19
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ use std::fs;
77

88
/// List of directories in docs.rs's underlying storage (either the database or S3) containing a
99
/// subdirectory named after the crate. Those subdirectories will be deleted.
10-
static STORAGE_PATHS_TO_DELETE: &[&str] = &["rustdoc", "sources"];
10+
static LIBRARY_STORAGE_PATHS_TO_DELETE: &[&str] = &["rustdoc", "sources"];
11+
static BINARY_STORAGE_PATHS_TO_DELETE: &[&str] = &["sources"];
1112

1213
#[derive(Debug, thiserror::Error)]
1314
enum CrateDeletionError {
@@ -18,9 +19,15 @@ enum CrateDeletionError {
1819
pub fn delete_crate(ctx: &dyn Context, name: &str) -> Result<()> {
1920
let conn = &mut ctx.pool()?.get()?;
2021
let crate_id = get_id(conn, name)?;
21-
delete_crate_from_database(conn, name, crate_id)?;
22+
let is_library = delete_crate_from_database(conn, name, crate_id)?;
23+
// #899
24+
let paths = if is_library {
25+
LIBRARY_STORAGE_PATHS_TO_DELETE
26+
} else {
27+
BINARY_STORAGE_PATHS_TO_DELETE
28+
};
2229

23-
for prefix in STORAGE_PATHS_TO_DELETE {
30+
for prefix in paths {
2431
// delete the whole rustdoc/source folder for this crate.
2532
// it will include existing archives.
2633
let remote_folder = format!("{}/{}/", prefix, name);
@@ -45,19 +52,26 @@ pub fn delete_version(ctx: &dyn Context, name: &str, version: &str) -> Result<()
4552
let conn = &mut ctx.pool()?.get()?;
4653
let storage = ctx.storage()?;
4754

48-
delete_version_from_database(conn, name, version)?;
55+
let is_library = delete_version_from_database(conn, name, version)?;
56+
let paths = if is_library {
57+
LIBRARY_STORAGE_PATHS_TO_DELETE
58+
} else {
59+
BINARY_STORAGE_PATHS_TO_DELETE
60+
};
4961

50-
for prefix in STORAGE_PATHS_TO_DELETE {
62+
for prefix in paths {
5163
storage.delete_prefix(&format!("{}/{}/{}/", prefix, name, version))?;
5264
}
5365

5466
let local_archive_cache = &ctx.config()?.local_archive_cache_path;
55-
for archive_filename in &[
56-
rustdoc_archive_path(name, version),
57-
source_archive_path(name, version),
58-
] {
67+
let mut paths = vec![source_archive_path(name, version)];
68+
if is_library {
69+
paths.push(rustdoc_archive_path(name, version));
70+
}
71+
72+
for archive_filename in paths {
5973
// delete remove archive and remote index
60-
storage.delete_prefix(archive_filename)?;
74+
storage.delete_prefix(&archive_filename)?;
6175

6276
// delete eventually existing local indexes
6377
let local_index_file = local_archive_cache.join(&format!("{}.index", archive_filename));
@@ -92,7 +106,8 @@ const METADATA: &[(&str, &str)] = &[
92106
("doc_coverage", "release_id"),
93107
];
94108

95-
fn delete_version_from_database(conn: &mut Client, name: &str, version: &str) -> Result<()> {
109+
/// Returns whether this release was a library
110+
fn delete_version_from_database(conn: &mut Client, name: &str, version: &str) -> Result<bool> {
96111
let crate_id = get_id(conn, name)?;
97112
let mut transaction = conn.transaction()?;
98113
for &(table, column) in METADATA {
@@ -101,10 +116,12 @@ fn delete_version_from_database(conn: &mut Client, name: &str, version: &str) ->
101116
&[&crate_id, &version],
102117
)?;
103118
}
104-
transaction.execute(
105-
"DELETE FROM releases WHERE crate_id = $1 AND version = $2",
106-
&[&crate_id, &version],
107-
)?;
119+
let is_library: bool = transaction
120+
.query_one(
121+
"DELETE FROM releases WHERE crate_id = $1 AND version = $2 RETURNING is_library",
122+
&[&crate_id, &version],
123+
)?
124+
.get("is_library");
108125
transaction.execute(
109126
"UPDATE crates SET latest_version_id = (
110127
SELECT id FROM releases WHERE release_time = (
@@ -114,17 +131,24 @@ fn delete_version_from_database(conn: &mut Client, name: &str, version: &str) ->
114131
&[&crate_id],
115132
)?;
116133

117-
for prefix in STORAGE_PATHS_TO_DELETE {
134+
let paths = if is_library {
135+
LIBRARY_STORAGE_PATHS_TO_DELETE
136+
} else {
137+
BINARY_STORAGE_PATHS_TO_DELETE
138+
};
139+
for prefix in paths {
118140
transaction.execute(
119141
"DELETE FROM files WHERE path LIKE $1;",
120142
&[&format!("{}/{}/{}/%", prefix, name, version)],
121143
)?;
122144
}
123145

124-
transaction.commit().map_err(Into::into)
146+
transaction.commit()?;
147+
Ok(is_library)
125148
}
126149

127-
fn delete_crate_from_database(conn: &mut Client, name: &str, crate_id: i32) -> Result<()> {
150+
/// Returns whether any release in this crate was a library
151+
fn delete_crate_from_database(conn: &mut Client, name: &str, crate_id: i32) -> Result<bool> {
128152
let mut transaction = conn.transaction()?;
129153

130154
transaction.execute(
@@ -142,13 +166,19 @@ fn delete_crate_from_database(conn: &mut Client, name: &str, crate_id: i32) -> R
142166
)?;
143167
}
144168
transaction.execute("DELETE FROM owner_rels WHERE cid = $1;", &[&crate_id])?;
169+
let has_library = transaction
170+
.query_one(
171+
"SELECT BOOL_OR(releases.is_library) AS has_library FROM releases",
172+
&[],
173+
)?
174+
.get("has_library");
145175
transaction.execute("DELETE FROM releases WHERE crate_id = $1;", &[&crate_id])?;
146176
transaction.execute("DELETE FROM crates WHERE id = $1;", &[&crate_id])?;
147177

148178
// Transactions automatically rollback when not committing, so if any of the previous queries
149179
// fail the whole transaction will be aborted.
150180
transaction.commit()?;
151-
Ok(())
181+
Ok(has_library)
152182
}
153183

154184
#[cfg(test)]

0 commit comments

Comments
 (0)