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

Support extra-verbose builds #48943

Merged
merged 1 commit into from
Mar 17, 2018
Merged

Support extra-verbose builds #48943

merged 1 commit into from
Mar 17, 2018

Conversation

comex
Copy link
Contributor

@comex comex commented Mar 12, 2018

  • The bootstrap crate currently passes -v to Cargo if itself invoked with -vv. But Cargo supports -vv (to show build script output), so make bootstrap pass that if itself invoked with -vvv. (More specifically, pass N '-v's to Cargo if invoked with N+1 of them.)

  • bootstrap.py currently tries to pass on up to two '-v's to cargo when building bootstrap, but incorrectly ('-v' is marked as 'store_true', so argparse stores either False or True, ignoring multiple '-v's). Fix this, allow passing any number of '-v's, and make it consistent with bootstrap's invocation of Cargo (i.e. subtract one from the number of '-v's).

  • Also improve bootstrap.py's config.toml 'parsing' to support arbitrary verbosity levels, + allow command line to override it.

@pietroalbini pietroalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 12, 2018
@pietroalbini
Copy link
Member

Highfive failed to assign a reviewer.

r? @Mark-Simulacrum

@Mark-Simulacrum
Copy link
Member

r=me after fixing tidy:

[00:06:25] tidy error: /checkout/src/bootstrap/flags.rs:32: line longer than 100 chars

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 13, 2018
@@ -597,10 +597,8 @@ def build_bootstrap(self):
self.cargo()))
args = [self.cargo(), "build", "--manifest-path",
os.path.join(self.rust_root, "src/bootstrap/Cargo.toml")]
if self.verbose:
for _ in range(0, max(0, self.verbose - 1)):
Copy link
Member

Choose a reason for hiding this comment

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

In both Python 2 and 3, range(0, self.verbose - 1) will be empty when self.verbose - 1 is zero or negative, so the max(0, _) is unnecessary.

@@ -29,7 +29,7 @@ use cache::{Interned, INTERNER};

/// Deserialized version of all flags for this compile.
pub struct Flags {
pub verbose: usize, // verbosity level: 0 == not verbose, 1 == verbose, 2 == very verbose
pub verbose: usize, // verbosity level: 0 == not verbose, 1 == verbose, 2 == very verbose, 3 == omg so verbose
Copy link
Member

Choose a reason for hiding this comment

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

🤣

(This line is >100 chars BTW)

pub fn is_very_verbose(&self) -> bool {
self.verbosity > 1
pub fn cargo_verbosity_level(&self) -> usize {
if self.verbosity == 0 { 0 } else { self.verbosity - 1 }
Copy link
Member

Choose a reason for hiding this comment

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

This can be simply self.verbosity.saturating_sub(1)

Alternatively you could use this in src/bootstrap/builder.rs to entirely get rid of this function.

// if rustbuild is invoked with N `-v`s, we pass N-1 `-v`s to cargo.
for _ in 1..self.verbosity {
    cargo.arg("-v");
}

- The bootstrap crate currently passes -v to Cargo if itself invoked
with -vv.  But Cargo supports -vv (to show build script output), so make
bootstrap pass that if itself invoked with -vvv.  (More specifically,
pass N '-v's to Cargo if invoked with N+1 of them.)

- bootstrap.py currently tries to pass on up to two '-v's to cargo when
building bootstrap, but incorrectly ('-v' is marked as 'store_true', so
argparse stores either False or True, ignoring multiple '-v's).  Fix
this, allow passing any number of '-v's, and make it consistent with
bootstrap's invocation of Cargo (i.e. subtract one from the number of
'-v's).

- Also improve bootstrap.py's config.toml 'parsing' to support arbitrary
verbosity levels, + allow command line to override it.
@comex
Copy link
Contributor Author

comex commented Mar 16, 2018

Updated in light of review comments.

@Mark-Simulacrum
Copy link
Member

r? @kennytm

@kennytm
Copy link
Member

kennytm commented Mar 16, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Mar 16, 2018

📌 Commit ec49234 has been approved by kennytm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 16, 2018
@alexcrichton
Copy link
Member

@bors: rollup

kennytm added a commit to kennytm/rust that referenced this pull request Mar 17, 2018
Support extra-verbose builds

- The bootstrap crate currently passes -v to Cargo if itself invoked with -vv.  But Cargo supports -vv (to show build script output), so make bootstrap pass that if itself invoked with -vvv.  (More specifically, pass N '-v's to Cargo if invoked with N+1 of them.)

- bootstrap.py currently tries to pass on up to two '-v's to cargo when building bootstrap, but incorrectly ('-v' is marked as 'store_true', so argparse stores either False or True, ignoring multiple '-v's).  Fix this, allow passing any number of '-v's, and make it consistent with bootstrap's invocation of Cargo (i.e. subtract one from the number of '-v's).

- Also improve bootstrap.py's config.toml 'parsing' to support arbitrary verbosity levels, + allow command line to override it.
bors added a commit that referenced this pull request Mar 17, 2018
Rollup of 8 pull requests

- Successful merges: #48943, #48960, #48983, #49055, #49057, #49077, #49082, #49083
- Failed merges:
@bors bors merged commit ec49234 into rust-lang:master Mar 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants