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.

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

Always rebuild targets when using cargo-fix #5750

Closed
Closed
Show file tree
Hide file tree
Changes from 6 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
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub fn cli() -> App {
)
.after_help(
"\
This Cargo subcommmand will automatically take rustc's suggestions from
This Cargo subcommand will automatically take rustc's suggestions from
diagnostics like warnings and apply them to your source code. This is intended
to help automate tasks that rustc itself already knows how to tell you to fix!
The `cargo fix` subcommand is also being developed for the Rust 2018 edition
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/core/compiler/build_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ pub struct BuildConfig {
pub mode: CompileMode,
/// Whether to print std output in json format (for machine reading)
pub message_format: MessageFormat,
/// Force cargo to do a full rebuild and treat each target as changed.
pub force_rebuild: bool,
/// Output a build plan to stdout instead of actually compiling.
pub build_plan: bool,
/// Use Cargo itself as the wrapper around rustc, only used for `cargo fix`
Expand Down Expand Up @@ -79,6 +81,7 @@ impl BuildConfig {
release: false,
mode,
message_format: MessageFormat::Human,
force_rebuild: false,
build_plan: false,
cargo_as_rustc_wrapper: false,
extra_rustc_env: Vec::new(),
Expand Down
4 changes: 3 additions & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ fn compile<'a, 'cfg: 'a>(
) -> CargoResult<()> {
let bcx = cx.bcx;
let build_plan = bcx.build_config.build_plan;
let force_rebuild = bcx.build_config.force_rebuild;
if !cx.compiled.insert(*unit) {
return Ok(());
}
Expand All @@ -124,6 +125,7 @@ fn compile<'a, 'cfg: 'a>(
)
} else {
let (mut freshness, dirty, fresh) = fingerprint::prepare_target(cx, unit)?;

let work = if unit.mode.is_doc() {
rustdoc(cx, unit)?
} else {
Expand All @@ -133,7 +135,7 @@ fn compile<'a, 'cfg: 'a>(
let dirty = work.then(link_targets(cx, unit, false)?).then(dirty);
let fresh = link_targets(cx, unit, true)?.then(fresh);

if exec.force_rebuild(unit) {
if exec.force_rebuild(unit) || force_rebuild {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matklad I've not added a force_rebuild field to the BuildConfig struct, and from what I can tell this should be the right place to use it. Indeed, when I add a debug!("rebuilding {:?} (force? {:?})", unit, force_rebuild); here, my new test case triggers it and shows the correct data. But I don't get any diagnostics output! Do I… need to go deeper?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's working. I think the issue is that the second call to cargo fix won't display "fixing" because the files have already been fixed in the first call.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, damn, I copied the wrong example! Thank you!

freshness = Freshness::Dirty;
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ impl CompileFilter {
/// Generates all the base targets for the packages the user has requested to
/// compile. Dependencies for these targets are computed later in
/// `unit_dependencies`.
fn generate_targets<'a>(
pub fn generate_targets<'a>(
ws: &Workspace,
profiles: &Profiles,
packages: &[&'a Package],
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ pub fn fix(ws: &Workspace, opts: &mut FixOptions) -> CargoResult<()> {
));
let _started = lock_server.start()?;

opts.compile_opts.build_config.force_rebuild = true;

if opts.broken_code {
let key = BROKEN_CODE_ENV.to_string();
opts.compile_opts.build_config.extra_rustc_env.push((key, "1".to_string()));
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
pub use self::cargo_clean::{clean, CleanOptions};
pub use self::cargo_compile::{compile, compile_with_exec, compile_ws, CompileOptions};
pub use self::cargo_compile::{compile, compile_with_exec, compile_ws, CompileOptions, generate_targets};
pub use self::cargo_compile::{CompileFilter, FilterRule, Packages};
pub use self::cargo_read_manifest::{read_package, read_packages};
pub use self::cargo_run::run;
Expand Down
107 changes: 107 additions & 0 deletions tests/testsuite/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,3 +1068,110 @@ fn does_not_warn_about_dirty_ignored_files() {
execs().with_status(0),
);
}

#[test]
fn shows_warnings_on_second_run_without_changes() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
"#,
)
.file(
"src/lib.rs",
r#"
use std::default::Default;

pub fn foo() {
}
"#,
)
.build();

assert_that(
p.cargo("fix --allow-no-vcs"),
execs().with_status(0).with_stderr_contains("[..]warning: unused import[..]"),
);

assert_that(
p.cargo("fix --allow-no-vcs"),
execs().with_status(0).with_stderr_contains("[..]warning: unused import[..]"),
);
}

#[test]
fn shows_warnings_on_second_run_without_changes_on_multiple_targets() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"

[workspace]
"#,
)
.file(
"src/lib.rs",
r#"
pub fn a() -> u32 { let mut x = 3; x }
"#,
)
.file(
"src/main.rs",
r#"
fn main() { let mut x = 3; println!("{}", x); }
"#,
)
.file(
"tests/foo.rs",
r#"
#[test]
fn foo_test() {
let mut x = 3; println!("{}", x);
}
"#,
)
.file(
"tests/bar.rs",
r#"
#[test]
fn foo_test() {
let mut x = 3; println!("{}", x);
}
"#,
)
.file(
"examples/fooxample.rs",
r#"
fn main() {
let mut x = 3; println!("{}", x);
}
"#,
)
.build();

assert_that(
p.cargo("fix --allow-no-vcs --all-targets").env("__CARGO_FIX_YOLO", "1"),
execs().with_status(0)
.with_stderr_contains("[FIXING] src[/]lib.rs (1 fix)")
.with_stderr_contains("[FIXING] src[/]main.rs (1 fix)")
.with_stderr_contains("[FIXING] tests[/]foo.rs (1 fix)")
.with_stderr_contains("[FIXING] tests[/]bar.rs (1 fix)")
.with_stderr_contains("[FIXING] examples[/]fooxample.rs (1 fix)"),
);

assert_that(
p.cargo("fix --allow-no-vcs --all-targets").env("__CARGO_FIX_YOLO", "1"),
execs().with_status(0)
.with_stderr_contains("[FIXING] src[/]lib.rs (1 fix)")
.with_stderr_contains("[FIXING] src[/]main.rs (1 fix)")
.with_stderr_contains("[FIXING] tests[/]foo.rs (1 fix)")
.with_stderr_contains("[FIXING] tests[/]bar.rs (1 fix)")
.with_stderr_contains("[FIXING] examples[/]fooxample.rs (1 fix)"),
);
}