Skip to content

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

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

Merged
merged 3 commits into from
Oct 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -463,6 +463,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 @@ -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-"))) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

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 @@ -646,11 +651,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 @@ -1164,3 +1188,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