-
Notifications
You must be signed in to change notification settings - Fork 13.6k
pass build.npm from bootstrap to tidy and use it for npm install #144317
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
Conversation
There are changes to the cc @jieyouxu |
This comment has been minimized.
This comment has been minimized.
53219e8
to
666fadf
Compare
666fadf
to
2d1fccd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@@ -29,6 +29,7 @@ fn main() { | |||
let concurrency: NonZeroUsize = | |||
FromStr::from_str(&env::args().nth(4).expect("need concurrency")) | |||
.expect("concurrency must be a number"); | |||
let npm: PathBuf = env::args_os().nth(5).expect("need name/path of npm command").into(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to think about using clap here soon... 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to refactor how bootstrap passes args to internal tools, I would prefer we use something more type-safe and typo-safe than manually serializing and deserializing args at every process boundary, especially considering these tools aren't really meant to be called directly.
my immediate thought would be putting some types into build_helper
which get serialized into json blobs that get passed to tools and deserialized. I actually tried doing this in a limited form with compiletest all the way back in #135653, trying to reduce the amount of code that has to be kept in sync on both sides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if some people don't use tidy or compiletest outside of bootstrap 🤷 But if not, then yeah, we might make the interface be more machine processing friendly.
@bors delegate+ You can r=me once PR CI is green. |
✌️ @lolbinarycat, you can now approve this pull request! If @Kobzol told you to " |
…r=Kobzol pass build.npm from bootstrap to tidy and use it for npm install followup to rust-lang#142924 r? `@Kobzol`
CI failure in rollup, not sure if it's due to this PR? Feels unlikely but it's the only one that touched bootstrap. Sorry in advance if I'm misjudging. |
Nah, it's a different PR. Also occurred in different PR: #144382 (comment) @bors r=Kobzol |
…r=Kobzol pass build.npm from bootstrap to tidy and use it for npm install followup to rust-lang#142924 r? `@Kobzol`
Rollup of 15 pull requests Successful merges: - #132748 (get rid of some false negatives in rustdoc::broken_intra_doc_links) - #143374 (Unquerify extern_mod_stmt_cnum.) - #143838 (std: net: uefi: Add support to query connection data) - #144014 (don't link to the nightly version of the Edition Guide in stable lints) - #144094 (Ensure we codegen the main fn) - #144218 (Use serde for target spec json deserialize) - #144221 (generate elf symbol version in raw-dylib) - #144240 (Add more test case to check if the false note related to sealed trait suppressed) - #144247 (coretests/num: use ldexp instead of hard-coding a power of 2) - #144276 (Use less HIR in check_private_in_public.) - #144317 (pass build.npm from bootstrap to tidy and use it for npm install) - #144320 (rustdoc: avoid allocating a temp String for aliases in search index) - #144334 (rustc_resolve: get rid of unused rustdoc::span_of_fragments_with_expansion) - #144335 (Don't suggest assoc ty bound on non-angle-bracketed problematic assoc ty binding) - #144358 (Stop using the old `validate_attr` logic for stability attributes) r? `@ghost` `@rustbot` modify labels: rollup
…r=Kobzol pass build.npm from bootstrap to tidy and use it for npm install followup to rust-lang#142924 r? ``@Kobzol``
Rollup of 16 pull requests Successful merges: - #143374 (Unquerify extern_mod_stmt_cnum.) - #143838 (std: net: uefi: Add support to query connection data) - #144014 (don't link to the nightly version of the Edition Guide in stable lints) - #144094 (Ensure we codegen the main fn) - #144218 (Use serde for target spec json deserialize) - #144221 (generate elf symbol version in raw-dylib) - #144232 (Implement support for `become` and explicit tail call codegen for the LLVM backend) - #144240 (Add more test case to check if the false note related to sealed trait suppressed) - #144247 (coretests/num: use ldexp instead of hard-coding a power of 2) - #144276 (Use less HIR in check_private_in_public.) - #144278 (add Rev::into_inner) - #144317 (pass build.npm from bootstrap to tidy and use it for npm install) - #144320 (rustdoc: avoid allocating a temp String for aliases in search index) - #144334 (rustc_resolve: get rid of unused rustdoc::span_of_fragments_with_expansion) - #144335 (Don't suggest assoc ty bound on non-angle-bracketed problematic assoc ty binding) - #144358 (Stop using the old `validate_attr` logic for stability attributes) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 15 pull requests Successful merges: - #143374 (Unquerify extern_mod_stmt_cnum.) - #143838 (std: net: uefi: Add support to query connection data) - #144014 (don't link to the nightly version of the Edition Guide in stable lints) - #144094 (Ensure we codegen the main fn) - #144218 (Use serde for target spec json deserialize) - #144221 (generate elf symbol version in raw-dylib) - #144240 (Add more test case to check if the false note related to sealed trait suppressed) - #144247 (coretests/num: use ldexp instead of hard-coding a power of 2) - #144276 (Use less HIR in check_private_in_public.) - #144278 (add Rev::into_inner) - #144317 (pass build.npm from bootstrap to tidy and use it for npm install) - #144320 (rustdoc: avoid allocating a temp String for aliases in search index) - #144334 (rustc_resolve: get rid of unused rustdoc::span_of_fragments_with_expansion) - #144335 (Don't suggest assoc ty bound on non-angle-bracketed problematic assoc ty binding) - #144358 (Stop using the old `validate_attr` logic for stability attributes) r? `@ghost` `@rustbot` modify labels: rollup
followup to #142924
r? @Kobzol