Skip to content

Commit

Permalink
Auto merge of #7257 - RobMor:no-spaces-between-flags, r=alexcrichton
Browse files Browse the repository at this point in the history
Fix #7217: Add support for rustc-flags without spaces between flags and values

Hi, I believe this pull request contains a fix for issue #7217. This is my first pull request to any open source project, much less any Rust project. I'm not super familiar with Rust at the moment, so let me know if I should change anything. Also any help/advice you can give me about this PR would be much appreciated!

I do have some specific questions:
- Is the test I added worthy of being its own test? I basically copied the one above it and remove the spaces between the flags and the values. Should I have added on to the test above it instead?
- Would it be better if I directly unit-tested this function in some other test file?
- Is the `ureachable!` macro good style?
  • Loading branch information
bors committed Aug 19, 2019
2 parents 60b7bf0 + a82de17 commit cfdf00e
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 16 deletions.
41 changes: 25 additions & 16 deletions src/cargo/core/compiler/custom_build.rs
Expand Up @@ -514,29 +514,38 @@ impl BuildOutput {
.split(|c: char| c.is_whitespace())
.filter(|w| w.chars().any(|c| !c.is_whitespace()));
let (mut library_paths, mut library_links) = (Vec::new(), Vec::new());

while let Some(flag) = flags_iter.next() {
if flag != "-l" && flag != "-L" {
if flag.starts_with("-l") || flag.starts_with("-L") {
// Check if this flag has no space before the value as is
// common with tools like pkg-config
// e.g. -L/some/dir/local/lib or -licui18n
let (flag, mut value) = flag.split_at(2);
if value.len() == 0 {
value = match flags_iter.next() {
Some(v) => v,
None => failure::bail! {
"Flag in rustc-flags has no value in {}: {}",
whence,
value
},
}
}

match flag {
"-l" => library_links.push(value.to_string()),
"-L" => library_paths.push(PathBuf::from(value)),

// This was already checked above
_ => unreachable!(),
};
} else {
failure::bail!(
"Only `-l` and `-L` flags are allowed in {}: `{}`",
whence,
value
)
}
let value = match flags_iter.next() {
Some(v) => v,
None => failure::bail!(
"Flag in rustc-flags has no value in {}: `{}`",
whence,
value
),
};
match flag {
"-l" => library_links.push(value.to_string()),
"-L" => library_paths.push(PathBuf::from(value)),

// was already checked above
_ => failure::bail!("only -l and -L flags are allowed"),
};
}
Ok((library_paths, library_links))
}
Expand Down
59 changes: 59 additions & 0 deletions tests/testsuite/build_script.rs
Expand Up @@ -254,6 +254,65 @@ fn custom_build_script_rustc_flags() {
.run();
}

#[cargo_test]
fn custom_build_script_rustc_flags_no_space() {
let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "bar"
version = "0.5.0"
authors = ["wycats@example.com"]
[dependencies.foo]
path = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.file(
"foo/Cargo.toml",
r#"
[project]
name = "foo"
version = "0.5.0"
authors = ["wycats@example.com"]
build = "build.rs"
"#,
)
.file("foo/src/lib.rs", "")
.file(
"foo/build.rs",
r#"
fn main() {
println!("cargo:rustc-flags=-lnonexistinglib -L/dummy/path1 -L/dummy/path2");
}
"#,
)
.build();

p.cargo("build --verbose")
.with_stderr(
"\
[COMPILING] foo [..]
[RUNNING] `rustc --crate-name build_script_build foo/build.rs [..]
[RUNNING] `[..]build-script-build`
[RUNNING] `rustc --crate-name foo foo/src/lib.rs [..]\
-L dependency=[CWD]/target/debug/deps \
-L /dummy/path1 -L /dummy/path2 -l nonexistinglib`
[COMPILING] bar [..]
[RUNNING] `rustc --crate-name bar src/main.rs [..]\
-L dependency=[CWD]/target/debug/deps \
--extern foo=[..]libfoo-[..] \
-L /dummy/path1 -L /dummy/path2`
[FINISHED] dev [..]
",
)
.run();
}

#[cargo_test]
fn links_no_build_cmd() {
let p = project()
Expand Down

0 comments on commit cfdf00e

Please sign in to comment.