-
Notifications
You must be signed in to change notification settings - Fork 409
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
Update bindgen version & fix some cache issue #526
Conversation
`child::run` overrides the stdout config, so is misleading to to set it differently beforehand. `child::run` uses `spawn()`, which inherits stdin by default. So, no need to set stdin to default beforehand, particularly when the spawned program is non-interactive.
Issue rustwasm#277 - Affects running login, pack, and publish on Windows. `Command::new("npm")` launched `npm` with quotes, `"npm"`, causing a run-time error on Windows. Now, `Command::new` is wrapped by `child::new_command(program: &str)`. This prepends `cmd /c` to the program name if `cfg!(windows)`. See rustc: #42436, #42791, #44542
Windows fixes
This commit replaces the `slog` family of crates used by `wasm-pack` with the `log` crate plus `env_logger`. This also means that by default `wasm-pack` also won't create a `wasm-pack.log` file in the current directory. Enabling logging will now be done through `RUST_LOG=wasm_pack` instead of `-v` flags. Closes rustwasm#425
Alexcrichton log
@huangjj27 is this still a work in progress? we should figure out that failing test- lemme know if you want help! i'll make as a WIP until i hear back from you |
I don’t think the failing test caused by, my PR, as I only change the wasm-bindgen version. In other word, the code is finished and can be merged, with the failing test to be solved in an independent PR. |
I believe the older code has something wrong so it install wasm-bindgen from cargo, but now it can download the prebuilt version. fixes rustwasm#447
8432eda
to
d56a944
Compare
because the assert commaned seems to capture stdout in err string, work around the test faing by double the log counts
@ashleygwilliams I think the PR is ready,and @csmoe has reviewed my code,please check and merge |
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.
thank you so much for your efforts and patience!
closing in favor of #554 |
This fixes an accidental regression from rustwasm#554 which was in turn an accidental regression from rustwasm#526
This fixes an accidental regression from rustwasm#554 which was in turn an accidental regression from rustwasm#526
Make sure these boxes are checked! 📦✅
rustfmt
installedcargo fmt
on the code base before submittingcloses #519 and fixes #447