Skip to content

rewrite consistency check & make it execute fixes #1990

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

Merged
merged 4 commits into from
Feb 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ exclude = [
]

[features]
consistency_check = ["crates-index", "rayon"]
consistency_check = ["crates-index", "rayon", "itertools"]

[dependencies]
sentry = "0.29.0"
Expand Down Expand Up @@ -67,6 +67,7 @@ zip = {version = "0.6.2", default-features = false, features = ["bzip2"]}
bzip2 = "0.4.4"
serde_cbor = "0.11.1"
getrandom = "0.2.1"
itertools = { version = "0.10.5", optional = true}

# Async
tokio = { version = "1.0", features = ["rt-multi-thread", "signal", "macros"] }
Expand Down Expand Up @@ -96,6 +97,7 @@ percent-encoding = "2.2.0"

# NOTE: if you change this, also double-check that the comment in `queue_builder::remove_tempdirs` is still accurate.
tempfile = "3.1.0"
fn-error-context = "0.2.0"

# Templating
tera = { version = "1.5.0", features = ["builtins"] }
Expand All @@ -121,7 +123,6 @@ kuchiki = "0.8"
rand = "0.8"
mockito = "0.31.0"
test-case = "2.0.0"
fn-error-context = "0.2.0"
aws-smithy-client = { version = "0.53.0", features = ["test-util"]}
aws-smithy-http = "0.53.0"
indoc = "1.0.7"
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cratesfyi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ impl DatabaseSubcommand {

#[cfg(feature = "consistency_check")]
Self::Synchronize { dry_run } => {
docs_rs::utils::consistency::run_check(&mut *ctx.conn()?, &*ctx.index()?, dry_run)?;
docs_rs::utils::consistency::run_check(&ctx, dry_run)?;
}
}
Ok(())
Expand Down
87 changes: 45 additions & 42 deletions src/build_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::storage::Storage;
use crate::utils::{get_config, get_crate_priority, report_error, set_config, ConfigName};
use crate::{Config, Index, Metrics, RustwideBuilder};
use anyhow::Context;
use fn_error_context::context;

use tracing::{debug, error, info, warn};

Expand Down Expand Up @@ -69,6 +70,7 @@ impl BuildQueue {
Ok(())
}

#[context("error trying to add {name}-{version} to build queue")]
pub fn add_crate(
&self,
name: &str,
Expand Down Expand Up @@ -342,15 +344,16 @@ impl BuildQueue {
let yanked = change.yanked();
let unyanked = change.unyanked();
if let Some(release) = yanked.or(unyanked) {
// FIXME: https://github.com/rust-lang/docs.rs/issues/1934
// we sometimes have inconsistencies between our yank-state and
// the crates.io yank state, and we don't know why yet.
self.set_yanked(
// FIXME: delay yanks of crates that have not yet finished building
// https://github.com/rust-lang/docs.rs/issues/1934
if let Err(err) = self.set_yanked(
&mut conn,
release.name.as_str(),
release.version.as_str(),
yanked.is_some(),
);
) {
report_error(&err);
}

if let Err(err) =
cdn::queue_crate_invalidation(&mut *conn, &self.config, &release.name)
Expand All @@ -372,48 +375,48 @@ impl BuildQueue {
Ok(crates_added)
}

fn set_yanked(&self, conn: &mut postgres::Client, name: &str, version: &str, yanked: bool) {
#[context("error trying to set {name}-{version} to yanked: {yanked}")]
pub fn set_yanked(
&self,
conn: &mut postgres::Client,
name: &str,
version: &str,
yanked: bool,
) -> Result<()> {
let activity = if yanked { "yanked" } else { "unyanked" };

match conn
.execute(
"UPDATE releases
SET yanked = $3
FROM crates
WHERE crates.id = releases.crate_id
AND name = $1
AND version = $2
",
&[&name, &version, &yanked],
)
.with_context(|| format!("error while setting {}-{} to {}", name, version, activity))
{
Ok(rows) => {
if rows != 1 {
match self
.has_build_queued(name, version)
.context("error trying to fetch build queue")
{
Ok(false) => {
// the rustwide builder will fetch the current yank state from
// crates.io, so and missed update here will be fixed after the
// build is finished.
error!(
"tried to yank or unyank non-existing release: {} {}",
name, version
);
}
Ok(true) => {}
Err(err) => {
report_error(&err);
}
}
} else {
debug!("{}-{} {}", name, version, activity);
let rows = conn.execute(
"UPDATE releases
SET yanked = $3
FROM crates
WHERE crates.id = releases.crate_id
AND name = $1
AND version = $2",
&[&name, &version, &yanked],
)?;
if rows != 1 {
match self
.has_build_queued(name, version)
.context("error trying to fetch build queue")
{
Ok(false) => {
// the rustwide builder will fetch the current yank state from
// crates.io, so and missed update here will be fixed after the
// build is finished.
error!(
"tried to yank or unyank non-existing release: {} {}",
name, version
);
}
Ok(true) => {}
Err(err) => {
report_error(&err);
}
}
Err(err) => report_error(&err),
} else {
debug!("{}-{} {}", name, version, activity);
}
Ok(())
}

/// Builds the top package from the queue. Returns whether there was a package in the queue.
Expand Down
3 changes: 3 additions & 0 deletions src/db/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::error::Result;
use crate::storage::{rustdoc_archive_path, source_archive_path, Storage};
use crate::{Config, Context};
use anyhow::Context as _;
use fn_error_context::context;
use postgres::Client;
use std::fs;

Expand All @@ -16,6 +17,7 @@ enum CrateDeletionError {
MissingCrate(String),
}

#[context("error trying to delete crate {name} from database")]
pub fn delete_crate(
conn: &mut Client,
storage: &Storage,
Expand Down Expand Up @@ -52,6 +54,7 @@ pub fn delete_crate(
Ok(())
}

#[context("error trying to delete release {name}-{version} from database")]
pub fn delete_version(ctx: &dyn Context, name: &str, version: &str) -> Result<()> {
let conn = &mut ctx.pool()?.get()?;
let storage = ctx.storage()?;
Expand Down
2 changes: 1 addition & 1 deletion src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub use self::pool::{Pool, PoolClient, PoolError};

mod add_package;
pub mod blacklist;
mod delete;
pub mod delete;
pub(crate) mod file;
mod migrate;
mod pool;
Expand Down
7 changes: 1 addition & 6 deletions src/index/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,7 @@ impl Index {

#[cfg(feature = "consistency_check")]
pub(crate) fn crates(&self) -> Result<crates_index::Index> {
use tracing::debug;
// First ensure the index is up to date, peeking will pull the latest changes without
// affecting anything else.
debug!("Updating index");
self.diff()?.peek_changes()?;
debug!("Opening with `crates_index`");
tracing::debug!("Opening with `crates_index`");
// crates_index requires the repo url to match the existing origin or it tries to reinitialize the repo
let repo_url = self
.repository_url
Expand Down
53 changes: 10 additions & 43 deletions src/utils/consistency/data.rs
Original file line number Diff line number Diff line change
@@ -1,48 +1,15 @@
use std::{
cmp::PartialEq,
collections::BTreeMap,
fmt::{self, Debug, Display, Formatter},
};

#[derive(Default, Debug)]
pub(crate) struct Data {
pub(crate) crates: BTreeMap<CrateName, Crate>,
}

#[derive(PartialOrd, Ord, PartialEq, Eq, Clone, Default, Debug)]
pub(crate) struct CrateName(pub(crate) String);

#[derive(Default, Debug)]
pub(crate) struct Crate {
pub(crate) releases: BTreeMap<Version, Release>,
}

#[derive(PartialOrd, Ord, PartialEq, Eq, Clone, Default, Debug)]
pub(crate) struct Version(pub(crate) String);

#[derive(Default, Debug)]
pub(crate) struct Release {}

impl PartialEq<String> for CrateName {
fn eq(&self, other: &String) -> bool {
self.0 == *other
}
#[derive(Clone, PartialEq, Debug)]
pub(super) struct Crate {
pub(super) name: String,
pub(super) releases: Releases,
}

impl PartialEq<String> for Version {
fn eq(&self, other: &String) -> bool {
self.0 == *other
}
}
pub(super) type Crates = Vec<Crate>;

impl Display for CrateName {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
Display::fmt(&self.0, f)
}
}
pub(super) type Releases = Vec<Release>;

impl Display for Version {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
Display::fmt(&self.0, f)
}
#[derive(Clone, Debug, PartialEq)]
pub(super) struct Release {
pub(super) version: String,
pub(super) yanked: Option<bool>,
}
Loading