Skip to content

Unify no-library-target error into no-target warn#14163

Closed
ryoqun wants to merge 1 commit into
rust-lang:masterfrom
ryoqun:no-no-library-target-error
Closed

Unify no-library-target error into no-target warn#14163
ryoqun wants to merge 1 commit into
rust-lang:masterfrom
ryoqun:no-no-library-target-error

Conversation

@ryoqun

@ryoqun ryoqun commented Jun 28, 2024

Copy link
Copy Markdown

What does this PR try to resolve?

Fixes #10958
Fixes #15231

as far as i checked gitub issues, source code and git log, it seems that there's no strong reason for cargo to emit hard-errors, not warns, only if --lib is given. This is an inconsistent behavior compared to other filters like --bins.

And the no-library-target error predates the generic no-target warn.

All being said, i think it's acceptable to demote the lib error into a generic warn.

How should we test and review this PR?

Updated existing unit tests demonstrate the new expected behavioral change (error => warn demotion)

@rustbot

rustbot commented Jun 28, 2024

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 28, 2024
libs.push(proposal)
}
}
if !all_targets && libs.is_empty() && *lib == LibRule::True {

@ryoqun ryoqun Jun 28, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Seems the earliest ancestor of this error is this: https://github.com/rust-lang/cargo/pull/839/files#r1658124748 (around Nov 15, 2014)

};
append(bins, " `bins`,");
append(tests, " `tests`,");
append(examples, " `examples`,");

@ryoqun ryoqun Jun 28, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

seems the earliest ancestor of the following out-of-diff warn is this: https://github.com/rust-lang/cargo/pull/9549/files#diff-98da1b66b532e50e9bca971b453ee7de96b17436de1d7ef3824c9888d55be9bbR1189-R1193 (for some reason, i can't comment on the referenced pr....) (around Jun 10, 2021)

                return shell.warn(format!(
                    "target {}{} specified, but no targets matched; this is a no-op",
                    if miss_count > 1 { "filters" } else { "filter" },
                    filters,
                ));

@ryoqun ryoqun marked this pull request as ready for review June 28, 2024 04:40
@ryoqun ryoqun changed the title WIP Unify no-library-target error into no-target warn Unify no-library-target error into no-target warn Jun 28, 2024
Comment on lines -524 to -530
let mut append = |t: &FilterRule, s| {
if let FilterRule::All = *t {
miss_count += 1;
filters.push_str(s);
}
};

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

needed to move closer to the actually used place to avoid borrow check error.

@epage

epage commented Jun 28, 2024

Copy link
Copy Markdown
Contributor

Note that #10958 has not been marked Accepted and its generally best to build consensus on a solution in an issue and leave a PR for discussing the implementation.

@ryoqun

ryoqun commented Jul 2, 2024

Copy link
Copy Markdown
Author

Note that #10958 has not been marked Accepted and its generally best to build consensus on a solution in an issue and leave a PR for discussing the implementation.

thanks for the heads-up. I just replied there: #10958 (comment)

@bors

bors commented Jul 10, 2024

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #14226) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

rustbot commented Dec 20, 2024

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly 081d7ba) made this pull request unmergeable. Please resolve the merge conflicts.

@epage

epage commented Jan 28, 2026

Copy link
Copy Markdown
Contributor

As this is pending an agreed-to-design in #10958 , I'm going to close this for now.

@epage epage closed this Jan 28, 2026
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cargo test --lib should not error when no lib is present Provide an option to cargo check everything except benchmarks

4 participants