bugfix(command/build.rs): Cancel Escape

[huangjj27]

fix #390

Make sure these boxes are checked! 📦✅

• You have the latest version of rustfmt installed and have your
  cloned directory set to nightly

$ rustup override set nightly
$ rustup component add rustfmt-preview --toolchain nightly

• You ran rustfmt on the code base before submitting
• You reference which issue is being closed in the PR text

✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨

[ashleygwilliams]

so PathBuf doesn't have a default display on purpose- you'll likely want to use to_str or to_string_lossy (the previous can fail, the former is likely the better option here, though may sometimes print weird things).

[fitzgen]

so PathBuf doesn't have a default display on purpose- you'll likely want to use to_str or to_string_lossy (the previous can fail, the former is likely the better option here, though may sometimes print weird things).

I believe this is the preferred method for formatting paths: https://doc.rust-lang.org/nightly/std/path/struct.Path.html#method.display

[ashleygwilliams]

oh huh, good call @fitzgen i didn't even know that existed. i'm curious that if that exists why Display trait isn't formally impl'd but anyways, seems the right way to go (as best as i can tell that display function does the same as to_string_lossy but it's hard to actually say)

[huangjj27]

this fix may conflict with #389

[ashleygwilliams]

@huangjj27 thanks for this! one ask- could you please include test(s)?

[huangjj27]

@ashleygwilliams I would be glad to! but what kinds of tests should I add? Is it OK to just add a unit test to check the modified function? I tried to write a test, but failed figure out how to get the log when it builds successfully, in order to make some assertions on the log's content. Would you give me some tips?

[csmoe]

@huangjj27 add a test case here to check whether windows path output is unneccesarily escaped as this PR intended to fix. the existing testcases are good examples for you to create a test case such as pkg_path_output_well_formatted or any name you like.

[ashleygwilliams]

@huangjj27 if you would prefer i can also grab this PR and add the tests for you! just let me know what you'd like :)

[huangjj27]

Hi! I added a test for the change, but the test case seems to keep failing on installing wasm-bindgen, but the source code of test crate generated can be built using normal call of wasm-pack build, is that caused by something special in command::run_wasm_pack?

what the "source code of test crate generated" means the file generated by utils::fixture::js_hello_world()

here is the log:

Oct 11 22:10:22.496 ERRO Process exited with exit code: 1: wasm-bindgen failed to execute properly. stderr:

error: failed to read `target/wasm32-unknown-unknown/release/js_hello_world.wasm`
	caused by: 系统找不到指定的路径。 (os error 3)    #  (system can't find the file)

this error is likely caused by not finding the CARGO_MANIFEST_DIR variable in fixture::new, so the target path point to else where.

[huangjj27]

@ashleygwilliams thanks! I'm working on the test(s) and glad to do the job, but I need some hints and times to finish it (for I could use only about an hour at night during workdays)

[ashleygwilliams]

hrm. let me investigate! thanks @huangjj27 and no worries on taking your time, i just wanted to make sure you weren't stuck!

[huangjj27]

Hi, @fitzgen @ashleygwilliams , I just update a test case that can pass without redirecting $CARGO_TARGET_DIR of the temporary proejct. this can be merged after solving #408

[huangjj27]

It seems the test case triggered an panic
, which is not found at local

[huangjj27]

It's so weird that I passed the test on my windows but failed on appveyor. I even can't get the logs from appveyor. What should I do next?

[huangjj27]

It's so strange that the result of cargo fmt is totally different. The code pushed is formatted in local, it gave me a total reversed result it turns out that I have used a older edition of the formator.

[huangjj27]

---- build::it_format_out_dir_on_windows stdout ----
Created fixture at C:\projects\wasm-pack-071k0\target\t\.tmpReZwQh\wasm-pack
   [ERR]: There was a crate configuration error. Details:
thread 'build::it_format_out_dir_on_windows' panicked at 'directories in wasm-pack.log should be well formatted', tests\all\build.rs:56:5
stack backtrace:
failures:
    build::it_format_out_dir_on_windows
test result: FAILED. 34 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
error: test failed, to rerun pass '--test all'
Command exited with code 101

Hi, @ashleygwilliams , I get such error that never occur at local and I'm not sure if it's the appveyor that causes the problem. Would you re-trigger the CI?

[fitzgen]

I fixed the appveyor CI on master, so if you rebase, then it should start passing and we can merge this!

[fitzgen]

err, I suppose this hasn't had another round of review yet, so maybe not merge immediately after rebase :-p

[huangjj27]

I once thought that it's using git bash in my vscode that makes the test failed. but after I change to powershell as integrated terminal, all test passed again.

[huangjj27]

I finally figure out there is difference of the path between ch and local, and
I shouldn’t hard code much.

[ashleygwilliams]

this is great! thanks so much for sticking with it!