Handle target specific outputs such as .wasm or .dll.lib #4570

tatsuya6502 · 2017-10-03T12:45:33Z

Until now, Cargo does not have any knowledge about target-specific output files. It relies solely on rustc for this sort of information (rustc --print=file-names ...).

As a result, Cargo does not place some build artifacts (files) to target/{debug,release} directory. These files include *.wasm for wasm32-unknown-emscripten target and *.dll.lib for *-pc-windows-msvc cdylib target.

This commit will add such knowledge to Cargo so that *.wasm and *.dll.lib will be placed in target/{debug,release} directory.

EDIT: I added a summary of changes. Please read it for more details of changes.

IMPORTANT
Although I added test cases for both wasm32-unknown-emscripten and *-pc-windows-msvc cdylib targets, I did not do manual testing on wasm32-unknown-emscripten target as I do not have an environment with emscripten installed. It will be appreciated if anybody tests this change for wasm32-unknown-emscripten target. EDIT: Tested for both wasm32-unknown-emscripten and x86_64-pc-windows-msvc. Thanks @Herschel for the help.

Fixes rust-lang#4500, rust-lang#4535 Until now, Cargo does not have any knowledge about target-specific output files. It relies solely on rustc for this sort of information (`rustc --print=file-names ...`). As a result, Cargo does not place some build artifacts (files) to target/{debug,release} directory. These files include *.wasm for wasm32-unknown-emscripten target and *.dll.lib for *-pc-windows-msvc cdylib target. This commit will add such knowledge to Cargo so that *.wasm and *.dll.lib will be placed in target/{debug,release} directory.

rust-highfive · 2017-10-03T12:45:46Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

tatsuya6502 · 2017-10-04T11:49:35Z

Although I added test cases for both wasm32-unknown-emscripten and *-pc-windows-msvc cdylib targets, I did not do manual testing on wasm32-unknown-emscripten target as I do not have an environment with emscripten installed.

Please do not merge this; we found a problem with wasm target. See this comment for details.

Fixes rust-lang#4535 - Do not add metadata to wasm32 bin target because generated .js will refer corresponding .wasm. - Handle different usages of "-" and "_" in .js and .wasm file names. (e.g "foo-bar.js" vs. "foo_bar.wasm") - Change file mode of cargo_rustc/context.rs (100755 -> 100644)

tatsuya6502 · 2017-10-05T11:06:03Z

Please do not merge this; we found a problem with wasm target. See this comment for details.

I added one more commit to address the problem.

Now this PR is ready for merge.

tatsuya6502 · 2017-10-05T11:09:01Z

Summary of Changes

For bin target of wasm32-unknown-emscripten

Copy .wasm file from dep to its parent directory.
- If crate name contains hyphens (e.g. "wasm-test"), rustc/emcc will generate js with hyphens ("wasm-test.js") and wasm with underscores ("wasm_test.wasm"). The updated codes is aware of this and will copy the correct wasm file.
Remove metadata suffix from the target js and wasm files in dep. This is required because wasm file name is embedded in generated js file. (See this comment for details.)

Before this PR:

[package]
name = "wasm-test"
# ...

% tree target/wasm32-unknown-emscripten/
target/wasm32-unknown-emscripten/
└── debug
    ├── build
    ├── deps
    │   ├── wasm_test-0f5ba9409d843569.js
    │   ├── wasm_test-0f5ba9409d843569.wasm
    │   ├── wasm_test-0f5ba9409d843569.wasm.map
    │   └── wasm_test-0f5ba9409d843569.wast
    ├── examples
    ├── incremental
    ├── native
    ├── wasm-test.d
    └── wasm-test.js

6 directories, 6 files

After this PR:

% tree target/wasm32-unknown-emscripten/
target/wasm32-unknown-emscripten/
└── debug
    ├── build
    ├── deps
    │   ├── wasm-test.js         <== no metadata, hyphen
    │   ├── wasm_test.wasm       <== no metadata
    │   ├── wasm_test.wasm.map
    │   └── wasm_test.wast
    ├── examples
    ├── incremental
    ├── native
    ├── wasm_test.d
    ├── wasm-test.d
    ├── wasm-test.js
    └── wasm_test.wasm           <== added

6 directories, 8 files

For cdylib/dylib targets of *-pc-windows-msvc

Copy .dll.lib file from dep to its parent directory.

Before this PR:

[package]
name = "cdylib-test"
# ...

[lib]
crate-type = ["cdylib"]

PS C:\Users\tatsuya\Documents\cdylib-test> dir .\target\debug\deps\

    Directory: C:\Users\tatsuya\Documents\cdylib-test\target\debug\deps

Mode                LastWriteTime         Length Name
----                -------------         ------ ----
-a----        10/5/2017  06:20 PM         115200 cdylib_test.dll
-a----        10/5/2017  06:20 PM           2141 cdylib_test.dll.exp
-a----        10/5/2017  06:20 PM           3892 cdylib_test.dll.lib
-a----        10/5/2017  06:20 PM        1085440 cdylib_test.pdb

PS C:\Users\tatsuya\Documents\cdylib-test> dir .\target\debug\

    Directory: C:\Users\tatsuya\Documents\cdylib-test\target\debug

Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d-----        10/5/2017  06:20 PM                .fingerprint
d-----        10/5/2017  06:20 PM                build
d-----        10/5/2017  06:20 PM                deps
d-----        10/5/2017  06:20 PM                examples
d-----        10/5/2017  06:20 PM                incremental
d-----        10/5/2017  06:20 PM                native
-a----        10/5/2017  06:20 PM              0 .cargo-lock
-a----        10/5/2017  06:20 PM            119 cdylib_test.d
-a----        10/5/2017  06:20 PM         115200 cdylib_test.dll

After this PR:

PS C:\Users\tatsuya\Documents\cdylib-test> dir .\target\debug\deps\

    Directory: C:\Users\tatsuya\Documents\cdylib-test\target\debug\deps


Mode                LastWriteTime         Length Name
----                -------------         ------ ----
-a----        10/5/2017  06:28 PM         115200 cdylib_test.dll
-a----        10/5/2017  06:28 PM           2141 cdylib_test.dll.exp
-a----        10/5/2017  06:28 PM           3892 cdylib_test.dll.lib
-a----        10/5/2017  06:28 PM        1085440 cdylib_test.pdb


PS C:\Users\tatsuya\Documents\cdylib-test> dir .\target\debug\

    Directory: C:\Users\tatsuya\Documents\cdylib-test\target\debug


Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d-----        10/5/2017  06:28 PM                .fingerprint
d-----        10/5/2017  06:28 PM                build
d-----        10/5/2017  06:28 PM                deps
d-----        10/5/2017  06:28 PM                examples
d-----        10/5/2017  06:28 PM                incremental
d-----        10/5/2017  06:28 PM                native
-a----        10/5/2017  06:28 PM              0 .cargo-lock
-a----        10/5/2017  06:28 PM            119 cdylib_test.d
-a----        10/5/2017  06:28 PM         115200 cdylib_test.dll
-a----        10/5/2017  06:28 PM            123 cdylib_test.dll.d
-a----        10/5/2017  06:28 PM           3892 cdylib_test.dll.lib  <== added

alexcrichton

Nice! I've got just one small nit but otherwise this looks great to me and should be good to go. If you're extra enterprising (this is optional) I think we could tackle #4056 with this infrastructure as well, but again, that's optional!

alexcrichton · 2017-10-05T14:56:14Z

src/cargo/ops/cargo_rustc/context.rs

@@ -478,9 +481,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
        // doing this eventually.
        let __cargo_default_lib_metadata = env::var("__CARGO_DEFAULT_LIB_METADATA");
        if !unit.profile.test &&
-            (unit.target.is_dylib() || unit.target.is_cdylib()) &&
+            (unit.target.is_dylib() || unit.target.is_cdylib() ||
+                 (unit.target.is_bin() && self.target_triple().starts_with("wasm32-"))) &&


Could this be refactored into a dedicated method? Something like:

fn fails_to_work_with_mangled_filename(...) -> bool { // ... }

(just to make it a bit more clear as to the intent)

tatsuya6502 · 2017-10-05T16:08:46Z

Thank you for reviewing the codes.

If you're extra enterprising (this is optional) I think we could tackle #4056 with this infrastructure as well

Ah, alright, there is another. Sure. I will take that. I have macOS environment and will have some time to spare in coming weekend.

alexcrichton · 2017-10-05T20:48:59Z

Nice! My thinking is that we probably just need to lift up the .dSYM directory whenever debuginfo is enabled for a profile. Additionally we may need to disable mangling entirely, although I'm not sure if that's still required.

That actually reminds me, maybe we should lift up the pdb files produced with MSVC debug info as well?

Again though, both of these are optional, we can always tackle in future PRs :)

tatsuya6502 · 2017-10-09T13:07:25Z

I did not have enough time to update this PR last weekend. I was only able to reproduce the debugger problems on macOS and Windows MSVC; no time for updating the codes. I will try to work on them in tomorrow evening (It is Monday evening now in my time zone).

My impression is I can address macOS debugger problem in this PR in a couple of days, but not Windows one. I found the full-path to the PDB file is embedded in Windows binary, so I can't move the PDB to anywhere else. It seems there is a way to remove the full-path via a linker flag. But I guess we will need to change rustc's linker driver to achieve this(?)

alexcrichton · 2017-10-09T20:48:14Z

Ok no worries! Let's go ahead and get this in and iterate in-tree

@bors: r+

bors · 2017-10-09T20:48:15Z

📌 Commit f9a541b has been approved by alexcrichton

bors · 2017-10-09T20:48:22Z

⌛ Testing commit f9a541b with merge 463e850...

@Herschel

…ichton Handle target specific outputs such as .wasm or .dll.lib Fixes #4500, #4535 Until now, Cargo does not have any knowledge about target-specific output files. It relies solely on rustc for this sort of information (`rustc --print=file-names ...`). As a result, Cargo does not place some build artifacts (files) to target/{debug,release} directory. These files include *.wasm for wasm32-unknown-emscripten target and *.dll.lib for *-pc-windows-msvc cdylib target. This commit will add such knowledge to Cargo so that *.wasm and *.dll.lib will be placed in target/{debug,release} directory. **EDIT**: I added [a summary of changes](#4570 (comment)). Please read it for more details of changes. **IMPORTANT** Although I added test cases for both wasm32-unknown-emscripten and *-pc-windows-msvc cdylib targets, ~I did not do manual testing on wasm32-unknown-emscripten target as I do not have an environment with emscripten installed. It will be appreciated if anybody tests this change for wasm32-unknown-emscripten target.~ **EDIT**: Tested for both wasm32-unknown-emscripten and x86_64-pc-windows-msvc. Thanks @Herschel for the help.

bors · 2017-10-09T21:19:34Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 463e850 to master...

Uplift *.dSYM Fixed #4490. The solution is based on #4570. Simply adding `.dSYM` into `add_target_specific_suffixes` will cause cargo trying to actually run that `.dSYM` folder, so I've upgraded the `linkable` boolean into a 3-value enum `TargetFileType`, to tell `cargo run` and `cargo test` to avoid these debug symbol files. (I haven't checked if it can solve #4056, don't wanna mess with Spotlight 😝)

Due to rust-lang/cargo#4570, a `*.dll.lib` file is uplifted when building dynamic libraries on Windows. The current bootstrap code does not understand files with multiple extensions, and will instead assume `xxxx.dll` is the file name. This caused a `no output generated` error because it tries to search for `xxxx.dll-hash.lib` inside the `deps/` folder, while it should check for `xxxx-hash.dll.lib` instead. This PR is blocking rust-lang#45285 (Bump to 1.23 and update bootstrap).

…ck, r=alexcrichton rustbuild: Fix `no output generated` error for next bootstrap cargo. Due to rust-lang/cargo#4570, a `*.dll.lib` file is uplifted when building dynamic libraries on Windows. The current bootstrap code does not understand files with multiple extensions, and will instead assume `xxxx.dll` is the file name. This caused a `no output generated` error because it tries to search for `xxxx.dll-hash.lib` inside the `deps/` folder, while it should check for `xxxx-hash.dll.lib` instead. This PR is blocking rust-lang#45285, see rust-lang#45285 (comment) and the rest of the comments for detail.

rust-highfive assigned alexcrichton Oct 3, 2017

Revert the file mode on context.rs (100644 -> 100755)

84b3989

tatsuya6502 mentioned this pull request Oct 3, 2017

Cargo does not provide a .wasm file in the correct location when building for wasm32-unknown-emscripten #4535

Closed

alexcrichton reviewed Oct 5, 2017

View reviewed changes

kennytm mentioned this pull request Oct 7, 2017

Line numbers in backtrace still do not appear on macOS for Cargo projects #4490

Closed

bors merged commit f9a541b into rust-lang:master Oct 9, 2017

tatsuya6502 deleted the target-specific-artifacts branch October 10, 2017 10:53

kennytm mentioned this pull request Oct 13, 2017

Uplift *.dSYM #4616

Merged

kennytm mentioned this pull request Oct 24, 2017

Bump to 1.23 and update bootstrap rust-lang/rust#45285

Merged

kennytm mentioned this pull request Oct 24, 2017

rustbuild: Fix no output generated error for next bootstrap cargo. rust-lang/rust#45496

Merged

alexcrichton mentioned this pull request Oct 31, 2017

assertion when using cargo run with wasm32-unknown-emscripten #4689

Open

alexcrichton mentioned this pull request Nov 25, 2017

Regression in filename handling on wasm with tests #4750

Closed

ehuss added this to the 1.22.0 milestone Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle target specific outputs such as .wasm or .dll.lib #4570

Handle target specific outputs such as .wasm or .dll.lib #4570

tatsuya6502 commented Oct 3, 2017 •

edited

Loading

rust-highfive commented Oct 3, 2017

tatsuya6502 commented Oct 4, 2017

tatsuya6502 commented Oct 5, 2017

tatsuya6502 commented Oct 5, 2017

alexcrichton left a comment

alexcrichton Oct 5, 2017

tatsuya6502 commented Oct 5, 2017

alexcrichton commented Oct 5, 2017

tatsuya6502 commented Oct 9, 2017

alexcrichton commented Oct 9, 2017

bors commented Oct 9, 2017

bors commented Oct 9, 2017

bors commented Oct 9, 2017

Handle target specific outputs such as .wasm or .dll.lib #4570

Handle target specific outputs such as .wasm or .dll.lib #4570

Conversation

tatsuya6502 commented Oct 3, 2017 • edited Loading

rust-highfive commented Oct 3, 2017

tatsuya6502 commented Oct 4, 2017

tatsuya6502 commented Oct 5, 2017

tatsuya6502 commented Oct 5, 2017

Summary of Changes

For bin target of wasm32-unknown-emscripten

For cdylib/dylib targets of *-pc-windows-msvc

alexcrichton left a comment

Choose a reason for hiding this comment

alexcrichton Oct 5, 2017

Choose a reason for hiding this comment

tatsuya6502 commented Oct 5, 2017

alexcrichton commented Oct 5, 2017

tatsuya6502 commented Oct 9, 2017

alexcrichton commented Oct 9, 2017

bors commented Oct 9, 2017

bors commented Oct 9, 2017

bors commented Oct 9, 2017

tatsuya6502 commented Oct 3, 2017 •

edited

Loading