Skip to content
This repository was archived by the owner on Jun 1, 2019. It is now read-only.

Fix building binaries #40

Closed
jmatraszek wants to merge 4 commits into from
Closed

Fix building binaries #40

jmatraszek wants to merge 4 commits into from

Conversation

@jmatraszek
Copy link
Contributor

@jmatraszek jmatraszek commented Jan 26, 2017

Fixing. Was broken by 2930ab8

@steveklabnik
Copy link
Contributor

steveklabnik commented Jan 26, 2017

Actually, could you just remove the path lines? it should work without them. I like to only configure what's not conventional.

Thanks for this! Should have done a better job of double checking. I assumed that the "build" would have taken care of it...

@jmatraszek
Copy link
Contributor Author

jmatraszek commented Jan 26, 2017

Removed. However, this gives me a warning: warning: file found to be present in multiple build targets: /home/kuba/dev/contributors/src/main.rs -- do not know if this is something we should worry about + cannot test the binaries as currently cargo fails to compile the project (still figuring out why).

@steveklabnik
Copy link
Contributor

steveklabnik commented Jan 26, 2017

Hmm, that does seem bad. @alexcrichton do you have any recommendations for what this configuration is supposed to look like?

@jmatraszek
Copy link
Contributor Author

jmatraszek commented Jan 26, 2017 •
edited
Loading

In fact, achieved to build the project, but running bin (without path specified in Cargo.toml) is not
possible:

$ rustup run nightly cargo run --bin populate --     
warning: file found to be present in multiple build targets: /home/kuba/dev/contributors/src/main.rs
[...]
     Running `target/debug/populate`
Listening on http://0.0.0.0:1337

When path is specified it works ok:

$ rustup run nightly cargo run --bin populate -- 
    Finished dev [unoptimized + debuginfo] target(s) in 0.1 secs
     Running `target/debug/populate`
error: The following required arguments were not provided:
    --path <filepath>

Anyway, waiting for Alex to comment on this, if there is no better way to achieve this, then I can revert the last commit.

@alexcrichton
Copy link
Member

alexcrichton commented Jan 26, 2017

Weird! That sounds like it may be a bug in Cargo...

@steveklabnik
Copy link
Contributor

steveklabnik commented Jan 27, 2017

@jmatraszek okay so, let's revert that last commit to get rid of the warning for now, and maybe we'll end up filing a cargo bug.

sorry that this simple PR has taken so long.

@steveklabnik steveklabnik mentioned this pull request Jan 27, 2017
Merged
@steveklabnik
Copy link
Contributor

steveklabnik commented Jan 27, 2017

@jmatraszek actually, don't worry about it; I did some work on those binaries in #53 and so just rolled all the commits except the last one into there. Thank you so much again!

@jmatraszek jmatraszek mentioned this pull request Jan 29, 2017
Merged
@jmatraszek
Copy link
Contributor Author

jmatraszek commented Jan 29, 2017

Hi @steveklabnik, @alexcrichton,
Played with Cargo yesterday and I ended up fixing this: rust-lang/cargo#3609

bors added a commit to rust-lang/cargo that referenced this pull request Feb 2, 2017
@bors
Auto merge of #3609 - jmatraszek:build_proper_binary, r=alexcrichton
2b63c4f 
Fix building multiple binaries that do not have path spacified in Cargo.toml

When multiple binaries are specified in Cargo.toml, the binaries that do not have `path` specified are build from `src/main.rs`. Discovered here: rust-lang-nursery/thanks#40 (comment).

This was caused by setting for a binary a main layout here https://github.com/rust-lang/cargo/blob/master/src/cargo/util/toml.rs#L478, which caused `normalize` to not fallback to default binary path here https://github.com/rust-lang/cargo/blob/master/src/cargo/util/toml.rs#L1149 (as `bin.path` was always `Some("/path/to/main.rs")`.

Added a test and fixed this by not using `layout.main()`, so right now for bins without `path` specified we fallback to default path inferred from bin's name (e.g. `src/bin/foo.rs`), test if the file exists and only if it doesn't -- fallback to `src/main.rs`.

I do not have any knowledge about Cargo's design, so I am not sure if this is the proper place to test for file existence.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jmatraszek @steveklabnik @alexcrichton