Skip to content

Commit

Permalink
Auto merge of #4570 - tatsuya6502:target-specific-artifacts, r=alexcr…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
bors committed Oct 9, 2017
2 parents 8031ef1 + f9a541b commit 463e850
Show file tree
Hide file tree
Showing 2 changed files with 192 additions and 7 deletions.
68 changes: 61 additions & 7 deletions src/cargo/ops/cargo_rustc/context.rs
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,9 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
// - OSX encodes the dylib name in the executable
// - Windows rustc multiple files of which we can't easily link all of them
//
// No metadata for bin because of an issue
// - wasm32 rustc/emcc encodes the .wasm name in the .js (rust-lang/cargo#4535)
//
// Two exceptions
// 1) Upstream dependencies (we aren't exporting + need to resolve name conflict)
// 2) __CARGO_DEFAULT_LIB_METADATA env var
Expand All @@ -522,9 +525,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-"))) &&
unit.pkg.package_id().source_id().is_path() &&
!__cargo_default_lib_metadata.is_ok() {
!__cargo_default_lib_metadata.is_ok()
{
return None;
}

Expand Down Expand Up @@ -690,11 +695,30 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
};
match *crate_type_info {
Some((ref prefix, ref suffix)) => {
let filename = out_dir.join(format!("{}{}{}", prefix, stem, suffix));
let link_dst = link_stem.clone().map(|(ld, ls)| {
ld.join(format!("{}{}{}", prefix, ls, suffix))
});
ret.push((filename, link_dst, linkable));
let suffixes = add_target_specific_suffixes(
&self.target_triple(),
&crate_type,
suffix,
linkable,
);
for (suffix, linkable, should_replace_hyphens) in suffixes {
// wasm bin target will generate two files in deps such as
// "web-stuff.js" and "web_stuff.wasm". Note the different usages of
// "-" and "_". should_replace_hyphens is a flag to indicate that
// we need to convert the stem "web-stuff" to "web_stuff", so we
// won't miss "web_stuff.wasm".
let conv = |s: String| if should_replace_hyphens {
s.replace("-", "_")
} else {
s
};
let filename =
out_dir.join(format!("{}{}{}", prefix, conv(stem.clone()), suffix));
let link_dst = link_stem.clone().map(|(ld, ls)| {
ld.join(format!("{}{}{}", prefix, conv(ls), suffix))
});
ret.push((filename, link_dst, linkable));
}
Ok(())
}
// not supported, don't worry about it
Expand Down Expand Up @@ -1231,3 +1255,33 @@ fn parse_crate_type(

Ok(Some((prefix.to_string(), suffix.to_string())))
}

// (not a rustdoc)
// Return a list of 3-tuples (suffix, linkable, should_replace_hyphens).
//
// should_replace_hyphens will be used by the caller to replace "-" with "_"
// in a bin_stem. See the caller side (calc_target_filenames()) for details.
fn add_target_specific_suffixes(
target_triple: &str,
crate_type: &str,
suffix: &str,
linkable: bool,
) -> Vec<(String, bool, bool)> {
let mut ret = vec![(suffix.to_string(), linkable, false)];

// rust-lang/cargo#4500
if target_triple.ends_with("pc-windows-msvc") && crate_type.ends_with("dylib") &&
suffix == ".dll"
{
ret.push((".dll.lib".to_string(), false, false));
}

// rust-lang/cargo#4535
if target_triple.starts_with("wasm32-") && crate_type == "bin" &&
suffix == ".js"
{
ret.push((".wasm".to_string(), false, true));
}

ret
}
131 changes: 131 additions & 0 deletions tests/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3366,6 +3366,137 @@ fn cdylib_not_lifted() {
}
}

#[test]
fn cdylib_final_outputs() {
let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "foo-bar"
authors = []
version = "0.1.0"
[lib]
crate-type = ["cdylib"]
"#)
.file("src/lib.rs", "");

assert_that(p.cargo_process("build"), execs().with_status(0));

let files = if cfg!(windows) {
vec!["foo_bar.dll.lib", "foo_bar.dll"]
} else if cfg!(target_os = "macos") {
vec!["libfoo_bar.dylib"]
} else {
vec!["libfoo_bar.so"]
};

for file in files {
println!("checking: {}", file);
assert_that(&p.root().join("target/debug").join(&file), existing_file());
}
}

#[test]
fn wasm32_final_outputs() {
use cargo::core::{Shell, Target, Workspace};
use cargo::ops::{self, BuildConfig, Context, CompileMode, CompileOptions, Kind, Unit};
use cargo::util::Config;
use cargo::util::important_paths::find_root_manifest_for_wd;

let target_triple = "wasm32-unknown-emscripten";

let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "foo-bar"
authors = []
version = "0.1.0"
"#)
.file("src/main.rs", "fn main() {}");
p.build();

// We can't cross-compile the project to wasm target unless we have emscripten installed.
// So here we will not run `cargo build`, but just create cargo_rustc::Context and ask it
// what the target file names would be.

// Create various stuff required to build cargo_rustc::Context.
let shell = Shell::new();
let config = Config::new(shell, p.root(), p.root());
let root = find_root_manifest_for_wd(None, config.cwd()).expect("Can't find the root manifest");
let ws = Workspace::new(&root, &config).expect("Can't create workspace");

let opts = CompileOptions {
target: Some(target_triple),
.. CompileOptions::default(&config, CompileMode::Build)
};

let specs = opts.spec.into_package_id_specs(&ws).expect("Can't create specs");

let (packages, resolve) = ops::resolve_ws_precisely(
&ws,
None,
opts.features,
opts.all_features,
opts.no_default_features,
&specs,
).expect("Can't create resolve");

let build_config = BuildConfig {
requested_target: Some(target_triple.to_string()),
jobs: 1,
.. BuildConfig::default()
};

let pkgid = packages
.package_ids()
.filter(|id| id.name() == "foo-bar")
.collect::<Vec<_>>();
let pkg = packages.get(pkgid[0]).expect("Can't get package");

let target = Target::bin_target("foo-bar", p.root().join("src/main.rs"), None);

let unit = Unit {
pkg: &pkg,
target: &target,
profile: &ws.profiles().dev,
kind: Kind::Target,
};
let units = vec![unit];

// Finally, create the cargo_rustc::Context.
let mut ctx = Context::new(
&ws,
&resolve,
&packages,
&config,
build_config,
ws.profiles(),
).expect("Can't create context");

// Ask the context to resolve target file names.
ctx.probe_target_info(&units).expect("Can't probe target info");
let target_filenames = ctx.target_filenames(&unit).expect("Can't get target file names");

// Verify the result.
let mut expected = vec!["debug/foo-bar.js", "debug/foo_bar.wasm"];

assert_eq!(target_filenames.len(), expected.len());

let mut target_filenames = target_filenames
.iter()
.map(|&(_, ref link_dst, _)| link_dst.clone().unwrap())
.collect::<Vec<_>>();
target_filenames.sort();
expected.sort();

for (expected, actual) in expected.iter().zip(target_filenames.iter()) {
assert!(
actual.ends_with(expected),
format!("{:?} does not end with {}", actual, expected)
);
}
}

#[test]
fn deterministic_cfg_flags() {
// This bug is non-deterministic
Expand Down

0 comments on commit 463e850

Please sign in to comment.