Rustc args: strings vs vectors & extracting link directories

aldanor commented 5 years ago

@laumann

We need to parse the flags in utility methods like .clean_rmeta() which proves to be quite painful, e.g. link flags could be either -Lfoo or -L foo or -Lnative=foo, etc. The current method of splitting by whitespace is wrong and generally dangerous. It would also be hard to handle paths with spaces correctly.

One possible suggestion: instead, store flags as Vec<String>, so we can at least skip parsing quotes, whitespace, etc; each flags and its (optional) value to be stored in each element. An example would be ["-Lfoo", "-L", "foo", "-Lnative=foo"]. This would obviously be a breaking change.

Another option would be to still store flags as strings but only perform cleaning in the "default" locations (those added by the .link_deps()), but then the user folders would be ignored.

Finally, yet another option: still store flags as strings, but store link flags separately, link link_flags. It's also a little weird since then there would be an overlap.

aldanor commented 5 years ago

Related: #148

laumann commented 5 years ago

Yes, I agree, splitting by whitespace is dangerous and wrong, I had this discussion in #81.

We concluded at that time that what should ideally be changed is Config.target_rustcflags from Option<String> to Option<Vec<String>> (or Option<Vec<Path>> or something similar). This kind of change should be submitted for rustc's compiletest, because I'd prefer for this implementation and the original not to diverge too much.

But maybe we can sidestep the problem, by internally storing them somewhere else/outside of Config and just set Config.target_rustcflags when necessary? Would that make sense?

laumann commented 5 years ago

Manishearth / compiletest-rs

Rustc args: strings vs vectors & extracting link directories #149