Skip to content

Commit

Permalink
Resolve: Be less strict while offline.
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Apr 2, 2019
1 parent 6bdb9d3 commit 2378a9b
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 20 deletions.
6 changes: 6 additions & 0 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,12 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
/// eventually be returned from `wait_for_download`. Returns `Some(pkg)` if
/// the package is ready and doesn't need to be downloaded.
pub fn start(&mut self, id: PackageId) -> CargoResult<Option<&'a Package>> {
Ok(self
.start_inner(id)
.chain_err(|| format!("failed to download `{}`", id))?)
}

fn start_inner(&mut self, id: PackageId) -> CargoResult<Option<&'a Package>> {
// First up see if we've already cached this package, in which case
// there's nothing to do.
let slot = self
Expand Down
47 changes: 35 additions & 12 deletions src/cargo/sources/registry/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ impl<'cfg> RegistryIndex<'cfg> {
ret.reserve(contents.lines().count());
let lines = contents.lines().map(|s| s.trim()).filter(|l| !l.is_empty());

let online = !self.config.cli_unstable().offline;
// Attempt forwards-compatibility on the index by ignoring
// everything that we ourselves don't understand, that should
// allow future cargo implementations to break the
Expand All @@ -215,11 +214,7 @@ impl<'cfg> RegistryIndex<'cfg> {
return None;
}
};
if online || load.is_crate_downloaded(summary.package_id()) {
Some((summary, locked))
} else {
None
}
Some((summary, locked))
}));

Ok(())
Expand Down Expand Up @@ -274,17 +269,43 @@ impl<'cfg> RegistryIndex<'cfg> {
yanked_whitelist: &HashSet<PackageId>,
f: &mut dyn FnMut(Summary),
) -> CargoResult<()> {
if self.config.cli_unstable().offline {
if self.query_inner_with_online(dep, load, yanked_whitelist, f, false)? != 0 {
return Ok(());
}
// If offline, and there are no matches, try again with online.
// This is necessary for dependencies that are not used (such as
// target-cfg or optional), but are not downloaded. Normally the
// build should succeed if they are not downloaded and not used,
// but they still need to resolve. If they are actually needed
// then cargo will fail to download and an error message
// indicating that the required dependency is unavailable while
// offline will be displayed.
}
self.query_inner_with_online(dep, load, yanked_whitelist, f, true)?;
Ok(())
}

fn query_inner_with_online(
&mut self,
dep: &Dependency,
load: &mut dyn RegistryData,
yanked_whitelist: &HashSet<PackageId>,
f: &mut dyn FnMut(Summary),
online: bool,
) -> CargoResult<usize> {
let source_id = self.source_id;
let name = dep.package_name().as_str();
let summaries = self.summaries(name, load)?;
let summaries = summaries
.iter()
.filter(|&(summary, yanked)| {
!yanked || {
log::debug!("{:?}", yanked_whitelist);
log::debug!("{:?}", summary.package_id());
yanked_whitelist.contains(&summary.package_id())
}
(online || load.is_crate_downloaded(summary.package_id()))
&& (!yanked || {
log::debug!("{:?}", yanked_whitelist);
log::debug!("{:?}", summary.package_id());
yanked_whitelist.contains(&summary.package_id())
})
})
.map(|s| s.0.clone());

Expand All @@ -308,9 +329,11 @@ impl<'cfg> RegistryIndex<'cfg> {
_ => true,
});

let mut count = 0;
for summary in summaries {
f(summary);
count += 1;
}
Ok(())
Ok(count)
}
}
84 changes: 76 additions & 8 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,77 @@ fn main(){
.run();
}

#[test]
fn offline_unused_target_dep() {
// -Z offline with a target dependency that is not used and not downloaded.
Package::new("unused_dep", "1.0.0").publish();
Package::new("used_dep", "1.0.0").publish();
let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.1.0"
[dependencies]
used_dep = "1.0"
[target.'cfg(unused)'.dependencies]
unused_dep = "1.0"
"#,
)
.file("src/lib.rs", "")
.build();
// Do a build that downloads only what is necessary.
p.cargo("build")
.with_stderr_contains("[DOWNLOADED] used_dep [..]")
.with_stderr_does_not_contain("[DOWNLOADED] unused_dep [..]")
.run();
p.cargo("clean").run();
// Build offline, make sure it works.
p.cargo("build -Z offline")
.masquerade_as_nightly_cargo()
.run();
}

#[test]
fn offline_missing_optional() {
Package::new("opt_dep", "1.0.0").publish();
let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.1.0"
[dependencies]
opt_dep = { version = "1.0", optional = true }
"#,
)
.file("src/lib.rs", "")
.build();
// Do a build that downloads only what is necessary.
p.cargo("build")
.with_stderr_does_not_contain("[DOWNLOADED] opt_dep [..]")
.run();
p.cargo("clean").run();
// Build offline, make sure it works.
p.cargo("build -Z offline")
.masquerade_as_nightly_cargo()
.run();
p.cargo("build -Z offline --features=opt_dep")
.masquerade_as_nightly_cargo()
.with_stderr(
"\
[ERROR] failed to download `opt_dep v1.0.0`
Caused by:
can't make HTTP request in the offline mode
",
)
.with_status(101)
.run();
}

#[test]
fn incompatible_dependencies() {
Package::new("bad", "0.1.0").publish();
Expand Down Expand Up @@ -1282,14 +1353,11 @@ fn compile_offline_while_transitive_dep_not_cached() {
.with_status(101)
.with_stderr(
"\
error: no matching package named `baz` found
location searched: registry `[..]`
required by package `bar v0.1.0`
... which is depended on by `foo v0.0.1 ([CWD])`
As a reminder, you're using offline mode (-Z offline) \
which can sometimes cause surprising resolution failures, \
if this error is too confusing you may wish to retry \
without the offline flag.",
[ERROR] failed to download `baz v1.0.0`
Caused by:
can't make HTTP request in the offline mode
",
)
.run();
}
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/support/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ pub fn alt_api_url() -> Url {
///
/// p.cargo("run").with_stdout("24").run();
/// ```
#[must_use]
pub struct Package {
name: String,
vers: String,
Expand Down

0 comments on commit 2378a9b

Please sign in to comment.