Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rustc: filter out empty linker args #10749

Merged
merged 1 commit into from
Dec 1, 2013
Merged

rustc: filter out empty linker args #10749

merged 1 commit into from
Dec 1, 2013

Conversation

Blei
Copy link
Contributor

@Blei Blei commented Dec 1, 2013

This is inspired by a mystifying linker failure when using pkg-config to
generate the linker args: pkg-config produces output that ends in a
space, thus resulting in an empty linker argument.

Also added some updates to the concerning error messages that helped
spotting this bug.

@alexcrichton
Copy link
Member

Can you add a test for this?

@Blei
Copy link
Contributor Author

Blei commented Dec 1, 2013

Added a test, but can't test it unfortunately, as make check fails before running the run-make tests (unrelated failure). This test works when run manually.

@@ -0,0 +1,6 @@
-include ../tools.mk
# Notice the space in the end, this emulates the output of pkg-config
RUSTC_FLAGS = --link-args "-lc "
Copy link
Member

Choose a reason for hiding this comment

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

This can also just be a normal run-pass test, using the compile-flags directive (docs), i.e.

<license block>

// Notice the space at the end, this emulates the output of pkg-config
// compile-flags:--linkargs "-lc "

fn main() {}

I'd guess that that is nicer than adding a Makefile etc, but I don't really know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, that's much easier. Thanks, I'll update!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I tried that. Guess who has a problem with quoted comand line parameters: the test runner. I'm not sure if it's worth it to fix that or if the current makefile test suffices.

@alexcrichton
Copy link
Member

Could you squash these two commits together? Otherwise looks good to me!

@Blei
Copy link
Contributor Author

Blei commented Dec 1, 2013

Squashed and force-pushed.

This is inspired by a mystifying linker failure when using `pkg-config` to
generate the linker args: `pkg-config` produces output that ends in a
space, thus resulting in an empty linker argument.

Also added some updates to the concerning error messages that helped
spotting this bug.
bors added a commit that referenced this pull request Dec 1, 2013
This is inspired by a mystifying linker failure when using `pkg-config` to
generate the linker args: `pkg-config` produces output that ends in a
space, thus resulting in an empty linker argument.

Also added some updates to the concerning error messages that helped
spotting this bug.
@bors bors closed this Dec 1, 2013
@bors bors merged commit 32688f8 into rust-lang:master Dec 1, 2013
@Blei Blei deleted the fix-linker-args branch December 2, 2013 10:22
flip1995 pushed a commit to flip1995/rust that referenced this pull request May 5, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants