Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

Support analyzing a specific binary in project#373

Merged
nrc merged 3 commits intorust-lang:masterfrom
Xanewok:bin-target
Jun 25, 2017
Merged

Support analyzing a specific binary in project#373
nrc merged 3 commits intorust-lang:masterfrom
Xanewok:bin-target

Conversation

@Xanewok
Copy link
Copy Markdown
Contributor

@Xanewok Xanewok commented Jun 22, 2017

Added build_bin configuration, which allows to specify which crate should be compiled and analyzed. When both build_lib and build_bin are specified, build_lib is preferred.

let current_package = ws.current().unwrap();
let targets = current_package.targets();

let bins = {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Declared like this, at this scope, because CompileFilter accepts a slice of names that has to live at least as long as CompileOptions, Workspace

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems odd - the outer block shouldn't introduce any new scopes here?

Copy link
Copy Markdown
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

Basically looks good, some minor points inline

let current_package = ws.current().unwrap();
let targets = current_package.targets();

let bins = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems odd - the outer block shouldn't introduce any new scopes here?

src/build.rs Outdated
} else {
let mut bins = targets.iter().filter(|x| x.is_bin());
let bin = bins.find(|x| x.name() == rls_config.build_bin);
match bin { None => vec![], Some(bin) => vec![bin.name().to_owned()]}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

style nit - use multiple lines

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we should warn the user if they specify a bin which we then don't use. Not sure how best to do that - at least we should have a debug! message, but maybe we should also fail compilation with a message?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, right! While I worked on this, I implemented also returning notifications from build phase and passing them to the user, but I thought I can't use return any meaningful message for now (e.g. for implicit/detected targets by cargo), so I just separated that and didn't include it. But we definitely can here, as bin target has to be explicitly named in the manifest file! The notification bit is here, I can push PR with that later. For now is debug! okay?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yep, debug is fine for now

@nrc nrc merged commit 0ef19bd into rust-lang:master Jun 25, 2017
@Xanewok Xanewok deleted the bin-target branch July 4, 2017 22:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants