Skip to content
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

Always pass --lib to cargo with link-args #40

Merged
merged 1 commit into from
Sep 19, 2017
Merged

Conversation

pascalj
Copy link
Contributor

@pascalj pascalj commented May 18, 2017

Cargo will throw an error when the target of the compilation is
ambiguous. This leads to an error when there's an additional main.rs,
e.g. for easy testing during development. Specifying --lib makes it
clear that we only want to build the library.

Before

~/.cargo/bin/cargo rustc --release -- -C link-args=-Wl,-undefined,dynamic_lookup
error: extra arguments to `rustc` can only be passed to one target,
consider filtering

After

~/.cargo/bin/cargo rustc --release --lib -- -C link-args=-Wl,-undefined,dynamic_lookup

Versions:

rustc 1.17.0 (56124baa9 2017-04-24)
cargo 0.18.0 (fe7b0cdcf 2017-04-24)
Darwin host 16.5.0 Darwin Kernel Version 16.5.0: Fri Mar  3 16:52:33 PST 2017; root:xnu-3789.51.2~3/RELEASE_X86_64 x86_64

Cargo will throw an error when the target of the compilation is
ambiguous. This leads to an error when there's an additional main.rs,
e.g. for easy testing during development. Specifying `--lib` makes it
clear that we only want to build the library.

Before

```
~/.cargo/bin/cargo rustc --release -- -C link-args=-Wl,-undefined,dynamic_lookup
error: extra arguments to `rustc` can only be passed to one target,
consider filtering
```

After

```
~/.cargo/bin/cargo rustc --release --lib -- -C link-args=-Wl,-undefined,dynamic_lookup
```
Copy link
Owner

@malept malept left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I'll look into it a bit more later, but for now I just have one question.

@@ -114,6 +114,7 @@ def cargo_rustc_args
args = []
unless config.dynamic_linker_flags == '' || config.target_os == 'mingw32'
args.push(
'--lib',
Copy link
Owner

Choose a reason for hiding this comment

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

Why isn't this outside of the conditional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cargo will only complain about this issue when handing additional flags to rustc. So it works just fine when no linker flags are passed.

@malept
Copy link
Owner

malept commented May 18, 2017

I think this sort of bugfix needs a(n integration) regression test, but this project isn't particularly set up for that yet. I'll take suggestions on how to do it, though.

@pascalj
Copy link
Contributor Author

pascalj commented May 19, 2017

Yes, I agree. One way would be to ship a minimal gem under /test and have it build by the test suite after installing cargo/rustc. Or have the gem online (as in rubygems.org) and use the current thermite version to install it. The first version is probably easier to maintain, but it's not exactly elegant. Do you have any other ideas?

@pascalj pascalj closed this May 19, 2017
@pascalj pascalj reopened this May 19, 2017
@malept malept merged commit c4eb696 into malept:master Sep 19, 2017
@malept
Copy link
Owner

malept commented Sep 19, 2017

Sorry for the extremely long delay! I appreciate the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants